diff mbox series

download: regression with USE_SOURCE_DIR and non-tarball packages?

Message ID 6F4802D5-49F3-47D2-AC55-8A738F96C124@plan44.ch
State New
Headers show
Series download: regression with USE_SOURCE_DIR and non-tarball packages? | expand

Commit Message

Lukas Zeller June 14, 2021, 12:11 p.m. UTC
Hello Petr,

a while ago (after switching projects from 19.07.3 to 19.07.6) I experienced unexpected rebuilds of packages. I asked about this in the forum [1] with no echo, so last week I dug deeper and think I found your commit from last november "4e19cbc - download: handle possibly invalid local tarballs" [2] causing what I see.

I'm a build system newbie, so please apologize if I misunderstand things here. But it seems to me that this patch only works for packages that pull their upstream source from a tarball via download.pl, because only there a hash check prevents redownloads (causing rebuilds) in the last possible moment by exiting the script before the download would happen.

For packages with PKG_SOURCE_PROTO=git the new line in package.mk `$(DL_DIR)/$(FILE): FORCE` forces re-cloning the upstream repo and thus rebuilding the entire package every time. Also, the USE_SOURCE_DIR mechanism does not work any more.

Both git based packages and USE_SOURCE_DIR are central to my work (which is building automation devices based on OpenWrt, so most work is happening *in* my own packages, which are in git or, during hot development phases, via USE_SOURCE_DIR even directly linked dirs). I realize this is not a common situation for OpenWrt platform developers, but only for developers basing other stuff on OpenWrt.

As said, I don't understand the details of the build system enough to actually propose a proper patch right into the heart of it. But below is my current workaround to re-enable USE_SOURCE_DIR and git-based packages, hopefully not affecting anything else I can't see. So far, works for me, but definitely needs a thorough look by someone who actually understands the whole thing...

Lukas


[1] https://forum.openwrt.org/t/does-19-07-6-trigger-package-downloads-rebuilds-more-frequently-than-19-07-3/92471
[2] http://lists.openwrt.org/pipermail/openwrt-devel/2020-November/032235.html

Comments

Petr Štetiar June 15, 2021, 8:05 a.m. UTC | #1
Lukas Zeller <luz@plan44.ch> [2021-06-14 14:11:27]:

Hi,

> For packages with PKG_SOURCE_PROTO=git the new line in package.mk
> `$(DL_DIR)/$(FILE): FORCE` forces re-cloning the upstream repo and thus
> rebuilding the entire package every time. 

yes, it would do that in case PKG_MIRROR_HASH is invalid and this is wanted
behavior. That issue should be visible when building in verbose mode with V=s.

> Also, the USE_SOURCE_DIR mechanism does not work any more.

And what doesn't work exactly? I've just checked it and it works just fine for me:

 $ git clone git://git.openwrt.org/project/libubox /tmp/libubox
 $ make package/libubox/clean
 $ make package/libubox/prepare USE_SOURCE_DIR=/tmp/libubox

 $ time make package/libubox/compile V=s &> /dev/null

 real	0m8.241s
 user	0m7.179s
 sys	0m1.643s

 $ time make package/libubox/compile V=s &> /dev/null

 real	0m4.608s
 user	0m3.692s
 sys	0m1.251s

 $ time make package/libubox/compile V=s &> /dev/null

 real	0m4.625s
 user	0m3.740s
 sys	0m1.239s

-- ynezz
Lukas Zeller June 15, 2021, 2:52 p.m. UTC | #2
Hi,

> On 15 Jun 2021, at 10:05, Petr Štetiar <ynezz@true.cz> wrote:
> [...] yes, it would do that in case PKG_MIRROR_HASH is invalid [...]

Thanks, that helped a lot! I ran the exact same tests with libubox and as those worked, I realized that the difference between the packages I tried and libubox is that the latter has (of course) a PKG_MIRROR_HASH.

The packages I wanted to work with did not have a PKG_MIRROR_HASH at all, because under development and not yet released (PKG_SOURCE_VERSION:=master).

> [...] That issue should be visible when building in verbose mode with V=s.

Only if there is a PKG_MIRROR_HASH in the makefile. Otherwise, the repo is just re-cloned every time without an indication why (I now found that make package/xxx/check would have revealed it)

But finally I found out about PKG_MIRROR_HASH=skip, which seems to be the solution for packages under development. It is not documented as such, but as a step for updating the hash.

Would it make sense if I try to add a note explaining this under "Working on local application source" in the "Creating Packages" wiki page?

Lukas
Henrique de Moraes Holschuh June 16, 2021, 7:14 p.m. UTC | #3
On 15/06/2021 11:52, Lukas Zeller wrote:
>> On 15 Jun 2021, at 10:05, Petr Štetiar <ynezz@true.cz> wrote:
>> [...] yes, it would do that in case PKG_MIRROR_HASH is invalid [...]
> 
> Thanks, that helped a lot! I ran the exact same tests with libubox and as those worked, I realized that the difference between the packages I tried and libubox is that the latter has (of course) a PKG_MIRROR_HASH.

Keep in mind that the calculation of PKG_MIRROR_HASH is not really as 
stable and reproducible as it should be.

E.g. hashes calculated under Ubuntu 18.04LTS will fail horribly when you 
try to build under Debian 10 stable.

> The packages I wanted to work with did not have a PKG_MIRROR_HASH at all, because under development and not yet released (PKG_SOURCE_VERSION:=master).

Yeah, we had the same problem here...

> But finally I found out about PKG_MIRROR_HASH=skip, which seems to be the solution for packages under development. It is not documented as such, but as a step for updating the hash.

... and I didn't know about the =skip.  Will track that one down, it 
looks like it would help our workflow as well.

> Would it make sense if I try to add a note explaining this under "Working on local application source" in the "Creating Packages" wiki page?

Please do!
diff mbox series

Patch

--- a/include/package.mk
+++ b/include/package.mk
@@ -177,6 +177,12 @@  define Build/Exports/Default
 endef
 Build/Exports=$(Build/Exports/Default)

+ifneq ($(wildcard $(PKG_BUILD_DIR)/.source_dir),)
+  FORCE_DL:=
+else
+  FORCE_DL=$(if $(findstring git,$(PKG_SOURCE_PROTO)),,$(DL_DIR)/$(FILE): FORCE)
+endif
+
 define Build/CoreTargets
   STAMP_PREPARED:=$$(STAMP_PREPARED)
   STAMP_CONFIGURED:=$$(STAMP_CONFIGURED)
@@ -185,7 +191,7 @@  define Build/CoreTargets
   $(call Build/Autoclean)
   $(call DefaultTargets)

-  $(DL_DIR)/$(FILE): FORCE
+  $(FORCE_DL)

   download:
 	$(foreach hook,$(Hooks/Download),