diff mbox

[OpenWrt-Devel,12/14] target/sdk: Fix handling of already built packages

Message ID 1451800982-112057-13-git-send-email-openwrt@daniel.thecshore.com
State Rejected
Headers show

Commit Message

Daniel Dickinson Jan. 3, 2016, 6:03 a.m. UTC 
From: Daniel Dickinson <openwrt@daniel.thecshore.com>

When using SDK default to avoiding rebuilding packages previously built
(within same variant of a source package; alternate variants don't cause
isses so do allow them) and in addition avoid installing from feeds
packages previously built.  In both cases this may be overridden
by a force option on the command line.

For feeds install it is -f, and for building the force is
FORCE_SDK="<space separated lists of package variants to build>",
where package variant is just source package name for non
varianted packages and <source_package>-<variant> for varianted
packages

Signed-off-by: Daniel Dickinson <openwrt@daniel.thecshore.com>
---
 Makefile            |  5 +++--
 include/kernel.mk   | 37 ++++++++++++++++++++++---------------
 include/package.mk  | 29 ++++++++++++++++++++++++++---
 include/subdir.mk   |  2 +-
 package/Makefile    |  2 ++
 scripts/feeds       | 11 ++++++++++-
 target/sdk/Makefile |  5 +++++
 7 files changed, 69 insertions(+), 22 deletions(-)

Comments

Felix Fietkau Jan. 3, 2016, 2:51 p.m. UTC | #1 
On 2016-01-03 07:03, openwrt@daniel.thecshore.com wrote:
> From: Daniel Dickinson <openwrt@daniel.thecshore.com>
> 
> When using SDK default to avoiding rebuilding packages previously built
> (within same variant of a source package; alternate variants don't cause
> isses so do allow them) and in addition avoid installing from feeds
> packages previously built.  In both cases this may be overridden
> by a force option on the command line.
> 
> For feeds install it is -f, and for building the force is
> FORCE_SDK="<space separated lists of package variants to build>",
> where package variant is just source package name for non
> varianted packages and <source_package>-<variant> for varianted
> packages
> 
> Signed-off-by: Daniel Dickinson <openwrt@daniel.thecshore.com>
I think this is heading in the wrong direction, since it's somewhat
fragile. Instead, I'd like to see changes that make it possible to ship
the SDK without staging_dir/target-*, and allow cleaning it.

- Felix
Daniel Dickinson Jan. 3, 2016, 3:36 p.m. UTC | #2 
Actually, in keeping with your comment about rather having a fork of the 
source in SDKs, I'm thinking it would be better to rework all of this, 
so that SDK's automatically ship the source, and figure out a better 
mechanism for managing the recording of what is already built (probably 
just a plain text file) which can be easily 'overridden' by removing the 
record (or taking an approach similar to feeds.conf.default, only with a 
generated file instead of one in the source tree).

I hadn't thought of the issue of people shipping binary-only SDK's and 
think that having the SDK have a copy of the source (but that doesn't 
get in the way easy updates to the source) would be preferrable; my 
object with the SDK was to build only what's needed, when it's needed, 
rather than as means to ship binaries.  I was thinking more in terms of 
internal common code, but see how it could easily get abused, in it's 
current form.

Regards,

Daniel

On 03/01/16 09:51 AM, Felix Fietkau wrote:
> On 2016-01-03 07:03, openwrt@daniel.thecshore.com wrote:
>> From: Daniel Dickinson <openwrt@daniel.thecshore.com>
>>
>> When using SDK default to avoiding rebuilding packages previously built
>> (within same variant of a source package; alternate variants don't cause
>> isses so do allow them) and in addition avoid installing from feeds
>> packages previously built.  In both cases this may be overridden
>> by a force option on the command line.
>>
>> For feeds install it is -f, and for building the force is
>> FORCE_SDK="<space separated lists of package variants to build>",
>> where package variant is just source package name for non
>> varianted packages and <source_package>-<variant> for varianted
>> packages
>>
>> Signed-off-by: Daniel Dickinson <openwrt@daniel.thecshore.com>
> I think this is heading in the wrong direction, since it's somewhat
> fragile. Instead, I'd like to see changes that make it possible to ship
> the SDK without staging_dir/target-*, and allow cleaning it.
>
> - Felix
>
Felix Fietkau Jan. 3, 2016, 4:21 p.m. UTC | #3 
On 2016-01-03 16:36, Daniel Dickinson wrote:
> Actually, in keeping with your comment about rather having a fork of the 
> source in SDKs, I'm thinking it would be better to rework all of this, 
> so that SDK's automatically ship the source, and figure out a better 
> mechanism for managing the recording of what is already built (probably 
> just a plain text file) which can be easily 'overridden' by removing the 
> record (or taking an approach similar to feeds.conf.default, only with a 
> generated file instead of one in the source tree).
> 
> I hadn't thought of the issue of people shipping binary-only SDK's and 
> think that having the SDK have a copy of the source (but that doesn't 
> get in the way easy updates to the source) would be preferrable; my 
> object with the SDK was to build only what's needed, when it's needed, 
> rather than as means to ship binaries.  I was thinking more in terms of 
> internal common code, but see how it could easily get abused, in it's 
> current form.
I think the SDK should behave just like the normal build system. The
problem with relying on prebuilt stuff in the staging dir (or even the
build dir) is that quite often absolute path names leak into various
places of the build, making it impossible to relocate.

IMHO the only sane way to deal with that is to stop relying on prebuilt
stuff in build_dir or staging_dir/target-* entirely.

- Felix
diff mbox

Patch

diff --git a/Makefile b/Makefile
index a12e3ea..91c4779 100644
--- a/Makefile
+++ b/Makefile
@@ -41,8 +41,9 @@  else
 $(toolchain/stamp-install): $(tools/stamp-install)
 $(target/stamp-compile): $(toolchain/stamp-install) $(tools/stamp-install) $(BUILD_DIR)/.prepared
 $(package/stamp-compile): $(target/stamp-compile) $(package/stamp-cleanup)
-$(package/stamp-install): $(package/stamp-compile)
-$(target/stamp-install): $(package/stamp-compile) $(package/stamp-install)
+$(package/stamp-markforsdk): $(package/stamp-compile)
+$(package/stamp-install): $(package/stamp-compile) $(package/stamp-markforsdk)
+$(target/stamp-install): $(package/stamp-compile) $(package/stamp-markforsdk) $(package/stamp-install)
 
 printdb:
 	@true
diff --git a/include/kernel.mk b/include/kernel.mk
index 878a366..cce6130 100644
--- a/include/kernel.mk
+++ b/include/kernel.mk
@@ -179,9 +179,15 @@  $(call KernelPackage/$(1)/config)
 
   $(call KernelPackage/depends)
 
-  ifneq ($(if $(filter-out %=y %=n %=m,$(KCONFIG)),$(filter m y,$(foreach c,$(filter-out %=y %=n %=m,$(KCONFIG)),$($(c)))),.),)
-    ifneq ($(if $(SDK),$(filter-out $(LINUX_DIR)/%.ko,$(FILES)),$(strip $(FILES))),)
-      define Package/kmod-$(1)/install
+  NOBUILD:=$(if $(filter-out $(FORCE_SDK),$(PKG_NAME)$(if $(BUILD_VARIANT),-$(BUILD_VARIANT))),\
+	$(if $(wildcard $(TOPDIR)/pkgstamp/$(PKG_NAME)$(if $(BUILD_VARIANT),-$(BUILD_VARIANT))),\
+		1 \
+	) \
+  )
+  ifeq ($(NOBUILD),)
+    ifneq ($(if $(filter-out %=y %=n %=m,$(KCONFIG)),$(filter m y,$(foreach c,$(filter-out %=y %=n %=m,$(KCONFIG)),$($(c)))),.),)
+      ifneq ($(if $(SDK),$(filter-out $(LINUX_DIR)/%.ko,$(FILES)),$(strip $(FILES))),)
+        define Package/kmod-$(1)/install
 		  @for mod in $$(call version_filter,$$(FILES)); do \
 			if grep -q "$$$$$$$${mod##$(LINUX_DIR)/}" "$(LINUX_DIR)/modules.builtin"; then \
 				echo "NOTICE: module '$$$$$$$$mod' is built-in."; \
@@ -195,22 +201,23 @@  $(call KernelPackage/$(1)/config)
 		  done;
 		  $(call ModuleAutoLoad,$(1),$$(1),$(AUTOLOAD))
 		  $(call KernelPackage/$(1)/install,$$(1))
-      endef
-    endif
-  $(if $(CONFIG_PACKAGE_kmod-$(1)),
-    else
-      compile: $(1)-disabled
-      $(1)-disabled:
+        endef
+      endif
+    $(if $(CONFIG_PACKAGE_kmod-$(1)),
+      else
+        compile: $(1)-disabled
+        $(1)-disabled:
 		@echo "WARNING: kmod-$(1) is not available in the kernel config - generating empty package" >&2
 
-      define Package/kmod-$(1)/install
+        define Package/kmod-$(1)/install
 		true
-      endef
-  )
-  endif
-  $$(eval $$(call BuildPackage,kmod-$(1)))
+        endef
+    )
+    endif
+    $$(eval $$(call BuildPackage,kmod-$(1)))
 
-  $$(IPKG_kmod-$(1)): $$(wildcard $$(FILES))
+    $$(IPKG_kmod-$(1)): $$(wildcard $$(FILES))
+  endif
 endef
 
 version_filter=$(if $(findstring @,$(1)),$(shell $(SCRIPT_DIR)/metadata.pl version_filter $(KERNEL_PATCHVER) $(1)),$(1))
diff --git a/include/package.mk b/include/package.mk
index 6538afe..57d004f 100644
--- a/include/package.mk
+++ b/include/package.mk
@@ -7,7 +7,7 @@ 
 
 __package_mk:=1
 
-all: $(if $(DUMP),dumpinfo,compile)
+all: $(if $(DUMP),dumpinfo,compile markforsdk)
 
 PKG_BUILD_DIR ?= $(BUILD_DIR)/$(PKG_NAME)$(if $(PKG_VERSION),-$(PKG_VERSION))
 PKG_INSTALL_DIR ?= $(PKG_BUILD_DIR)/ipkg-install
@@ -65,6 +65,11 @@  STAMP_INSTALLED:=$(STAGING_DIR)/stamp/.$(PKG_NAME)$(if $(BUILD_VARIANT),.$(BUILD
 STAGING_FILES_LIST:=$(PKG_NAME)$(if $(BUILD_VARIANT),.$(BUILD_VARIANT),).list
 
 define CleanStaging
+	$(if $(PKG_NAME), \
+		rm -f $(STAGING_DIR)/pkgstamp-current/$(PKG_NAME)$(if $(BUILD_VARIANT),-$(BUILD_VARIANT)) \
+	,\
+		rm -rf $(STAGING_DIR)/pkgstamp-current \
+	)
 	rm -f $(STAMP_INSTALLED)
 	@-(\
 		cd "$(STAGING_DIR)"; \
@@ -214,6 +219,10 @@  define Build/DefaultTargets
 	rm -rf $(TMP_DIR)/stage-$(PKG_NAME)
 	touch $$@
 
+   markforsdk:
+	mkdir -p $(STAGING_DIR)/pkgstamp-current
+	touch $(STAGING_DIR)/pkgstamp-current/$(PKG_NAME)$(if $(BUILD_VARIANT),-$(BUILD_VARIANT))
+
   ifdef Build/InstallDev
     compile: $(STAMP_INSTALLED)
   endif
@@ -262,10 +271,23 @@  endif
     $(foreach target, \
       $(if $(Package/$(1)/targets),$(Package/$(1)/targets), \
         $(if $(PKG_TARGETS),$(PKG_TARGETS), ipkg) \
-      ), $(BuildTarget/$(target)) \
+      ), \
+        $(if $(filter $(FORCE_SDK),$(PKG_NAME)$(if $(BUILD_VARIANT),-$(BUILD_VARIANT))), \
+		$(BuildTarget/$(target)) \
+	, \
+		$(if $(wildcard $(TOPDIR)/pkgstamp/$(PKG_NAME)$(if $(BUILD_VARIANT),-$(BUILD_VARIANT))),, \
+			$(BuildTarget/$(target)) \
+		) \
+	) \
     ) \
   )
-  $(if $(PKG_HOST_ONLY)$(DUMP),,$(call Build/DefaultTargets,$(1)))
+  $(if $(filter $(FORCE_SDK),$(PKG_NAME)$(if $(BUILD_VARIANT),-$(BUILD_VARIANT))), \
+	$(if $(PKG_HOST_ONLY)$(DUMP),,$(call Build/DefaultTargets,$(1))) \
+  , \
+	$(if $(wildcard $(TOPDIR)/pkgstamp/$(PKG_NAME)$(if $(BUILD_VARIANT),-$(BUILD_VARIANT))),, \
+		$(if $(PKG_HOST_ONLY)$(DUMP),,$(call Build/DefaultTargets,$(1))) \
+	) \
+  )
 endef
 
 define pkg_install_files
@@ -298,6 +320,7 @@  dumpinfo:
 download:
 prepare:
 configure:
+markforsdk:
 compile: prepare-package-install
 install: compile
 
diff --git a/include/subdir.mk b/include/subdir.mk
index 8dc9a78..7c65fdf 100644
--- a/include/subdir.mk
+++ b/include/subdir.mk
@@ -9,7 +9,7 @@  ifeq ($(MAKECMDGOALS),prereq)
   SUBTARGETS:=prereq
   PREREQ_ONLY:=1
 else
-  SUBTARGETS:=clean download prepare compile install update refresh prereq dist distcheck configure
+  SUBTARGETS:=clean download prepare compile markforsdk install update refresh prereq dist distcheck configure
 endif
 
 subtarget-default = $(filter-out ., \
diff --git a/package/Makefile b/package/Makefile
index aa5d522..4dba4c8 100644
--- a/package/Makefile
+++ b/package/Makefile
@@ -22,6 +22,7 @@  ifneq ($(IGNORE_ERRORS),)
   package-ignore-errors := $(if $(package-ignore-errors),$(package-ignore-errors),n m)
   $(curdir)/builddirs-ignore-download := $(foreach m,$(package-ignore-errors),$(package-$(m)-filter))
   $(curdir)/builddirs-ignore-compile := $(foreach m,$(package-ignore-errors),$(package-$(m)-filter))
+  $(curdir)/builddirs-ignore-markforsdk := $(foreach m,$(package-ignore-errors),$(package-$(m)-filter))
 endif
 
 ifdef CONFIG_USE_MKLIBS
@@ -186,6 +187,7 @@  $(curdir)/flags-install:= -j1
 $(eval $(call stampfile,$(curdir),package,prereq,.config))
 $(eval $(call stampfile,$(curdir),package,cleanup,$(TMP_DIR)/.build))
 $(eval $(call stampfile,$(curdir),package,compile,$(TMP_DIR)/.build))
+$(eval $(call stampfile,$(curdir),package,markforsdk,$(TMP_DIR)/.build))
 $(eval $(call stampfile,$(curdir),package,install,$(TMP_DIR)/.build))
 
 $(eval $(call subdir,$(curdir)))
diff --git a/scripts/feeds b/scripts/feeds
index 79b5284..6aca11f 100755
--- a/scripts/feeds
+++ b/scripts/feeds
@@ -452,6 +452,10 @@  sub install_target {
 	return do_install_target($target);
 }
 
+sub is_in_sdk($) {
+	return (-e "$ENV{TOPDIR}/pkgstamp/$_[0]");
+}
+
 sub install_package {
 	my $feed = shift;
 	my $name = shift;
@@ -469,7 +473,7 @@  sub install_package {
 	$feed or do {
 		$installed{$name} and return 0;
 		# TODO: check if it's already installed within ./package directory
-		$feed_src->{$name} or is_core_package($name) or warn "WARNING: No feed for package '$name' found, maybe it's already part of the standard packages?\n";
+		$feed_src->{$name} or is_core_package($name) or is_in_sdk($name) or ($name =~ /kmod-/) or warn "WARNING: No feed for package '$name' found and not in SDK, is this a dependency of a unselected package?\n";
 		return 0;
 	};
 
@@ -477,16 +481,21 @@  sub install_package {
 	my $cur = get_feed($feed->[1]);
 
 	my $pkg = $cur->{$name} or return 1;
+
 	$pkg->{name} or do {
 		$installed{$name} and return 0;
 		# TODO: check if this is an alias package, maybe it's known by another name
 		warn "WARNING: Package '$name' is not available in feed $feed->[1].\n";
 		return 0;
 	};
+
 	my $src = $pkg->{src};
 	my $type = $feed->[0];
 	$src or $src = $name;
 
+	# If the package is in the sdk and we don't want to override it, just return
+	!$force and is_in_sdk($name) and return 0;
+
 	# If it's a core package and we don't want to override, just return
 	!$force and is_core_package($src) and return 0;
 
diff --git a/target/sdk/Makefile b/target/sdk/Makefile
index 80a87a8..9b1c192 100644
--- a/target/sdk/Makefile
+++ b/target/sdk/Makefile
@@ -21,6 +21,7 @@  STAGING_SUBDIR_TOOLCHAIN := staging_dir/toolchain-$(ARCH)$(ARCH_SUFFIX)_gcc-$(GC
 
 EXCLUDE_DIRS:=*/ccache \
 	*/stamp \
+	*/pkgstamp-current \
 	*/stampfiles \
 	*/man \
 	*/info \
@@ -105,6 +106,10 @@  $(BIN_DIR)/$(SDK_NAME).tar.bz2: clean
 		$(TOPDIR)/package/Makefile \
 		$(SDK_BUILD_DIR)/package/
 
+	# Prevent feeds install and/or rebuilt of packages already built (unless
+	# forced).
+	mkdir -p $(SDK_BUILD_DIR)/$(STAGING_DIR_TARGET)/pkgstamp
+	-$(CP) $(STAGING_DIR)/pkgstamp-current/* $(SDK_BUILD_DIR)/pkgstamp/
 	-rm -f $(SDK_BUILD_DIR)/feeds.conf.default
 	$(if $(BASE_FEED),echo "$(BASE_FEED)" > $(SDK_BUILD_DIR)/feeds.conf.default)
 	if [ -f $(TOPDIR)/feeds.conf ]; then \