13039 2018-11-23 09:24:33 +0000 fetch2: PREMIRROR and SRC_URI with type https and parameter downloadfilename yields invalid url 2021-11-09 01:43:00 +0000 1 1 7 Build System, Metadata & Runtime BitBake bitbake 2.6 x86 x86_64 RESOLVED FIXED Medium+ normal 3.4 M4 1 jal2 kai.kang kai.kang poky.bs.watcher poky.watcher randy.macleod richard.purdie weaverjs --- --- No (bug/feature does not impact docs) oldest_to_newest 82303 0 jal2 2018-11-23 09:24:33 +0000 A SRC_URI with parameter downloadfilename matching a PREMIRROR entry gets the local basename appended to the original basename. Example: recipe for uthash contains SRC_URI= "https://github.com/troydhanson/uthash/archive/v2.0.2.tar.gz;downloadfilename=uthash-2.0.2.tar.gz" local.conf: PREMIRROR += "https://github.com/troydhanson/uthash/archive http://nexus:8081/repository/https.github.com/troydhanson/uthash/archive \n " bitbake -DD -c fetch uthash says: DEBUG: uthash-2.0.2-r0 do_fetch: For url ['http', 'nexus:8081', '/repository/iav-yocto-mirror/troydhanson/uthash/archive/v2.0.2.tar.gz/uthash-2.0.2.tar.gz', '', '', OrderedDict([('downloadfilename', 'uthash-2.0.2.tar.gz')])] comparing ['http', '.*', '/.*', '', '', OrderedDict()] to ['http', 'nexus:8081', '/repository/http.HOST/PATH', '', '', OrderedDict()] DEBUG: uthash-2.0.2-r0 do_fetch: basename uthash-2.0.2.tar.gz from ud.localpath /iav/yocto/fsl-community-bsp/downloads/uthash-2.0.2.tar.gz DEBUG: uthash-2.0.2-r0 do_fetch: adding basename uthash-2.0.2.tar.gz to /repository/http.github.com//troydhanson/uthash/archive/v2.0.2.tar.gz DEBUG: uthash-2.0.2-r0 do_fetch: For url http://nexus:8081/repository/iav-yocto-mirror/troydhanson/uthash/archive/v2.0.2.tar.gz/uthash-2.0.2.tar.gz;downloadfilename=uthash-2.0.2.tar.gz returning http://nexus:8081/repository/http.github.com/troydhanson/uthash/archive/v2.0.2.tar.gz/uthash-2.0.2.tar.gz;downloadfilename=uthash-2.0.2.tar.gz The additional lines above are due to this patch: diff --git a/bitbake/lib/bb/fetch2/__init__.py b/bitbake/lib/bb/fetch2/__init__.py index a83526a..559afb7 100644 --- a/bitbake/lib/bb/fetch2/__init__.py +++ b/bitbake/lib/bb/fetch2/__init__.py @@ -478,7 +478,9 @@ def uri_replace(ud, uri_find, uri_replace, replacements, d, mirrortarball=None): uri_decoded[5] = {} elif ud.localpath and ud.method.supports_checksum(ud): basename = os.path.basename(ud.localpath) + logger.debug(2,"basename %s from ud.localpath %s" % (basename, ud.localpath)) if basename and not result_decoded[loc].endswith(basename): + logger.debug(2,"adding basename %s to %s" % (basename, result_decoded[loc])) result_decoded[loc] = os.path.join(result_decoded[loc], basename) else: return None 91358 1 weaverjs 2021-09-07 23:09:54 +0000 A patch for this issue was submitted. https://git.openembedded.org/bitbake/commit/?id=8a3ff9f3eaf19d4258eb070c5dc230dface269b2 Additional bitbake-selftest tests were added as well: - https://git.openembedded.org/bitbake/commit/?id=8a3ff9f3eaf19d4258eb070c5dc230dface269b2 - https://git.openembedded.org/bitbake/commit/?id=5ba191a0407af9e652e3b86302dce3e952d6b587 91490 2 kai.kang 2021-09-22 01:52:16 +0000 I suppose it is an invalid bug. It save the tarball with the name of the value of downloadfilename rather than the basename of the url. In this case, the name of tarball in the dir downloads/ is uthash-2.0.2.tar.gz rather than 2.0.2.tar.gz. So when premirror is used, it must the check uthash-2.0.2.tar.gz in the premirrors but not the original value 2.0.2.tar.gz. With this patch, it fails to fetch the tarball from premirrors. It could be reduced by config: BB_NO_NETWORK = "1" BB_FETCH_PREMIRRORONLY = "1" So I suggest to revert this patch. 91495 3 richard.purdie 2021-09-22 08:43:00 +0000 See above comment 91599 4 weaverjs 2021-09-28 00:21:48 +0000 Sorry for the late reply. I'm just now seeing this. I'll look into it and it sounds like another test is required. This might be a new/different bug. The issue addressed was in regard to the incorrect url being generated in the case described in comment 1. 91646 5 4835 weaverjs 2021-10-02 21:48:23 +0000 Created attachment 4835 Proposed fetcher test This patch adds a fetcher test which checks that a previous download using downloadfilename will be found when BB_NO_NETWORK is enabled. 91647 6 weaverjs 2021-10-02 21:49:52 +0000 (In reply to comment #2) > I suppose it is an invalid bug. It save the tarball with the name of the > value of downloadfilename rather than the basename of the url. In this case, > the name of tarball in the dir downloads/ is uthash-2.0.2.tar.gz rather than > 2.0.2.tar.gz. This is the expected behavior. https://www.yoctoproject.org/docs/latest/bitbake-user-manual/bitbake-user-manual.html#var-bb-SRC_URI > So when premirror is used, it must the check > uthash-2.0.2.tar.gz in the premirrors but not the original value > 2.0.2.tar.gz. IIUC, the fetcher using a PREMIRRORS URL should not look for the downloadfilename. It should look for the specified artifact name (e.g., 2.0.2.tar.gz in this example) and upon downloading it, it should save it as downloadfilename. In fact, the tests that I submitted in comment #1 test just that. But your comments did bring to mind when BB_NO_NETWORK is enabled if the fetcher would find the downloadfilename from the previous download. I've attached a patch with a new fetcher test that I believe covers this case. If you could review this test I would appreciate it. Thank you. Scott 91659 7 kai.kang 2021-10-05 00:08:21 +0000 (In reply to comment #6) > (In reply to comment #2) > > I suppose it is an invalid bug. It save the tarball with the name of the > > value of downloadfilename rather than the basename of the url. In this case, > > the name of tarball in the dir downloads/ is uthash-2.0.2.tar.gz rather than > > 2.0.2.tar.gz. > > This is the expected behavior. > https://www.yoctoproject.org/docs/latest/bitbake-user-manual/bitbake-user- > manual.html#var-bb-SRC_URI Yes, it should save the tarball with the value of downloadfilename and it does that before. But the argument do not work any more. It saves with basename of the URL now. Regards, Kai 91660 8 weaverjs 2021-10-05 01:30:53 +0000 (In reply to comment #7) > (In reply to comment #6) > > (In reply to comment #2) > > > I suppose it is an invalid bug. It save the tarball with the name of the > > > value of downloadfilename rather than the basename of the url. In this case, > > > the name of tarball in the dir downloads/ is uthash-2.0.2.tar.gz rather than > > > 2.0.2.tar.gz. > > > > This is the expected behavior. > > https://www.yoctoproject.org/docs/latest/bitbake-user-manual/bitbake-user- > > manual.html#var-bb-SRC_URI > > Yes, it should save the tarball with the value of downloadfilename and it > does that before. But the argument do not work any more. It saves with > basename of the URL now. These three tests[1] were written to confirm that the downloadfilename is being used when the tarball is saved. Are you saying that the tests pass but when downlaodfilename is used in a recipe itself it's not working or the test is incorrect? Could you share with me how you are testing this? Scott [1] https://git.openembedded.org/bitbake/commit/?id=61db3e96530d650e098436fd086f0182d32998f7 91697 9 kai.kang 2021-10-08 06:13:28 +0000 Hi Scott, I am sorry that I did not make it clear. It fails to fetch the SRC which contains argument 'downloadfilename' when ONLY from premirror if the mirror is a download directory of a previous build. Project 1: 1 $ . poky/oe-init-build-env build-fetch-111 2 Add related layers that a recipe contains 'downloadfilename' argument. Take meta-networking/recipes-protocols/freediameter/freediameter_1.4.0.bb as an example. Update conf/bblayers.conf with /buildarea6/kkang/Yocto/meta-openembedded/meta-oe \ /buildarea6/kkang/Yocto/meta-openembedded/meta-python \ /buildarea6/kkang/Yocto/meta-openembedded/meta-networking \ 3 $ bitbake freediameter -c fetch 4 $ ls downloads/freeDiameter* downloads/freeDiameter-1.4.0.tar.gz downloads/freeDiameter-1.4.0.tar.gz.done Project 2: 1 $ . poky/oe-init-build-env build-fetch-222 2 update bblayer.conf follow the uppper step 3 update local.conf to only fetch from pre-mirror: BB_NO_NETWORK = "1" BB_FETCH_PREMIRRORONLY = "1" PREMIRRORS =+ "http://.*/.* file:///buildarea6/kkang/Yocto/build-fetch-111/downloads/ \n" 4 $ bitbake freediameter -c fetch ERROR: freediameter-1.4.0-r0 do_fetch: Network access disabled through BB_NO_NETWORK (or set indirectly due to use of BB_FETCH_PRE MIRRORONLY) but access requested with command /usr/bin/env wget -t 2 -T 30 --passive-ftp -O /buildarea6/kkang/Yocto/build-fetch-22 2/downloads/freeDiameter-1.4.0.tar.gz -P /buildarea6/kkang/Yocto/build-fetch-222/downloads 'http://www.freediameter.net/hg/freeDia meter/archive/1.4.0.tar.gz' (for url http://www.freediameter.net/hg/freeDiameter/archive/1.4.0.tar.gz;downloadfilename=freeDiamete r-1.4.0.tar.gz) ERROR: Logfile of failure stored in: /buildarea6/kkang/Yocto/build-fetch-222/tmp/work/qemux86_64-poky-linux/freediameter/1.4.0-r0/ temp/log.do_fetch.34939 ERROR: Task (/buildarea6/kkang/Yocto/meta-openembedded/meta-networking/recipes-protocols/freediameter/freediameter_1.4.0.bb:do_fet ch) failed with exit code '1' 5 revert the commit and rerun $ git log -1 --oneline 4939a4198234c8dab1348590663c4681edae8fc9 4939a41982 bitbake: bitbake: fetch2: fix premirror URI when downloadfilename defined $ git revert 4939a4198234c8dab1348590663c4681edae8fc9 $ bitbake freediameter -c fetch Then it works. 91701 10 weaverjs 2021-10-09 16:13:49 +0000 Hi Kai, Thank you for the detailed description and for taking the time to document the steps. That really helped me understand what was going on. The patch is fine and should not be reverted and let me explain why. As you know, the way this should work is that the SRC_URI defines the artifact that should be downloaded. In the case of freediameter, that artifact is named 1.4.0.tar.gz. The downloadfilename only defines what the artifact should be saved as once downloaded. In this case that name is: freeDiameter-1.4.0.tar.gz Regardless of where the fetcher downloads that artifact from, be it from another http server or in your case a local directory (file://) the fetcher should attempt to download the original artifact name as defined in the SRC_URI (1.4.0.tar.gz). In your test, the first download retrieved the 1.4.0.tar.gz artifact from http://www.freediameter.net and saved the file as freeDiameter-1.4.0.tar.gz to the /buildarea6/kkang/Yocto/build-fetch-111/downloads/ directory. This is correct. So in the second fetch, the fetcher correctly tried to download 1.4.0.tar.gz from the premirror you defined (file:///buildarea6/kkang/Yocto/build-fetch-111/downloads/) because the SRC_URI specified that artifact name but as you showed, only the freeDiameter-1.4.0.tar.gz file is saved there. So the fetcher correctly failed to find the 1.4.0.tar.gz in the defined premirror. The fetcher, after failing to download the artifact from any source, should then try to retrieve the previous download (using the downloadfilename) from DL_DIR. But because DL_DIR is now defined as /buildarea6/kkang/Yocto/build-fetch-222/downloads/ it isn't going to find it there because the DL_DIR is empty at this point. If you had pointed DL_DIR to your build-fetch-111 downloads directory the fetcher would have successfully found the downloadfilename there. What you found was another instance of this bz bug 13039 and by reverting the patch it restored the buggy behavior which adds the downloadfilename to the URL. Which is incorrect. The fetcher should not attempt to download the downnloadfilename. I completely understand why it appeared like the patch needed reverted because something that had been working was no longer working but that use case that you're testing should have always failed because the fetcher should not have tried to download the downloadfilename from the premirror. The fetcher should not attempt to download the downloadfilename from a SRC_URI, MIRROR, or a PREMIRROR. The fetcher should, though, look for the downloadfilename from the DL_DIR if none of the URIs could be used to successfully download the specified artifact. Which the fetcher does do correctly. If you want the premirror that you defined to work, then you'll want to provide the mirrored artifact that is specified by the SRC_URI (/buildarea6/kkang/Yocto/build-fetch-111/downloads/1.4.0.tar.gz). You can also define your premirror to point directly to the local artifact (using any name you want) which will override what was defined in the SRC_URI. PREMIRRORS =+ "http://www.freediameter.net/.* file:///buildarea6/kkang/Yocto/build-fetch-111/downloads/freeDiameter-1.4.0.tar.gz \n" This was a good test you created and I hope I've clarified why it correctly failed and why this bug should be closed. Scott 91897 11 randy.macleod 2021-11-06 18:40:59 +0000 I think Scott meant to assign this back to Kai. 91914 12 kai.kang 2021-11-09 01:43:00 +0000 It has been fixed by Qi: https://git.openembedded.org/bitbake/commit/?id=96c30007dc0b32eee2b15771daec7948bc9bfd97 So mark fixed. 4835 2021-10-02 21:48:23 +0000 2021-10-02 21:48:23 +0000 Proposed fetcher test 0001-add-fetcher-test-using-downloadfilename-and-no-network.patch text/plain 1660 weaverjs ZGlmZiAtLWdpdCBiaXRiYWtlL2xpYi9iYi90ZXN0cy9mZXRjaC5weSBiaXRiYWtlL2xpYi9iYi90 ZXN0cy9mZXRjaC5weQppbmRleCBhZjI5MmEyMTYzLi4wMmI4NTZhMmQzIDEwMDY0NAotLS0gYml0 YmFrZS9saWIvYmIvdGVzdHMvZmV0Y2gucHkKKysrIGJpdGJha2UvbGliL2JiL3Rlc3RzL2ZldGNo LnB5CkBAIC04ODcsNiArODg3LDIyIEBAIGNsYXNzIEZldGNoZXJOZXR3b3JrVGVzdChGZXRjaGVy VGVzdCk6CiAgICAgICAgIGZldGNoZXIuZG93bmxvYWQoKQogICAgICAgICBzZWxmLmFzc2VydEVx dWFsKG9zLnBhdGguZ2V0c2l6ZShzZWxmLmRsZGlyICsgIi9iaXRiYWtlLXYxLjAuMC50YXIuZ3oi KSwgNTc3NDkpCiAKKyAgICBAc2tpcElmTm9OZXR3b3JrKCkKKyAgICAjIFRoaXMgY2hlY2tzIHRo YXQgdGhlIGZldGNoZXIgZG9lcyBub3QgZmFpbCB0byBmaW5kIHRoZSBkb3dubG9hZGZpbGVuYW1l IGZyb20gYSBwcmV2aW91cyBkb3dubG9hZC4KKyAgICBkZWYgdGVzdF9mZXRjaF9wcmVtaXJyb3Jf c3BlY2lmeV9kb3dubG9hZGZpbGVuYW1lX2FuZF93aXRoX25vX25ldHdvcmsoc2VsZik6CisgICAg ICAgICMgZmV0Y2ggdGhlIGZpbGUgYW5kIHNhdmUgaXQgYXMgZG93bmxvYWRmaWxlbmFtZQorICAg ICAgICBzZWxmLmQuc2V0VmFyKCJQUkVNSVJST1JTIiwgImh0dHA6Ly9pbnZhbGlkLnlvY3RvcHJv amVjdC5vcmcvcmVsZWFzZXMvYml0YmFrZSBodHRwOi8vZG93bmxvYWRzLnlvY3RvcHJvamVjdC5v cmcvcmVsZWFzZXMvYml0YmFrZSIpCisgICAgICAgIGZldGNoZXIgPSBiYi5mZXRjaC5GZXRjaChb Imh0dHA6Ly9pbnZhbGlkLnlvY3RvcHJvamVjdC5vcmcvcmVsZWFzZXMvYml0YmFrZS9iaXRiYWtl LTEuMC50YXIuZ3o7ZG93bmxvYWRmaWxlbmFtZT1iaXRiYWtlLXYxLjAuMC50YXIuZ3oiXSwgc2Vs Zi5kKQorICAgICAgICBmZXRjaGVyLmRvd25sb2FkKCkKKyAgICAgICAgc2VsZi5hc3NlcnRFcXVh bChvcy5wYXRoLmdldHNpemUoc2VsZi5kbGRpciArICIvYml0YmFrZS12MS4wLjAudGFyLmd6Iiks IDU3NzQ5KQorICAgICAgICAjIGZldGNoIHRoZSBmaWxlIGFnYWluLCBidXQgdGhpcyB0aW1lIHdp dGggQkJfTk9fTkVUV09SSyBlbmFibGVkICh0aGUgZmV0Y2hlciB3aWxsIGZpbmQgZG93bmxvYWRm aWxlbmFtZSBmcm9tIHRoZSBjYWNoZSkKKyAgICAgICAgc2VsZi5kLnNldFZhcigiQkJfTk9fTkVU V09SSyIsICIxIikKKyAgICAgICAgZmV0Y2hlci5kb3dubG9hZCgpCisgICAgICAgICMgZmV0Y2gg dGhlIGZpbGUgYWdhaW4sIGJ1dCB0aGlzIHRpbWUgdGhlIGZldGNoZXIgd2lsbCBmYWlsIGJlY2F1 c2UgaXQgd2lsbCBub3QgZmluZCB0aGUgb3JpZ2luYWwgZG93bmxvYWRlZCBmaWxlCisgICAgICAg IGJiLnV0aWxzLnJlbmFtZShzZWxmLmRsZGlyICsgIi9iaXRiYWtlLXYxLjAuMC50YXIuZ3oiLCBz ZWxmLmRsZGlyICsgIi9iaXRiYWtlLXgueS56LnRhci5neiIpCisgICAgICAgIHdpdGggc2VsZi5h c3NlcnRSYWlzZXMoYmIuZmV0Y2gyLk5ldHdvcmtBY2Nlc3MpOgorICAgICAgICAgICAgZmV0Y2hl ci5kb3dubG9hZCgpCisKICAgICBAc2tpcElmTm9OZXR3b3JrKCkKICAgICBkZWYgZ2l0ZmV0Y2hl cihzZWxmLCB1cmwxLCB1cmwyKToKICAgICAgICAgZGVmIGNoZWNrcmV2aXNpb24oc2VsZiwgZmV0 Y2hlcik6Cg==