| Message ID | 20241120073755.17583-1-simon@sinic.eu |
|---|---|
| State | New |
| Headers | show |
| Series | package/pkg-generic.mk: add missing quotes | expand |
On Wed, Nov 20, 2024 at 08:37:55AM UTC, Simon Richter via buildroot wrote: > This allows <PKG>_SOURCE to contain characters like `&`. > > Signed-off-by: Simon Richter <simon@sinic.eu> It is a bad idea to have such name, but it makes sense. LGTM Reviewed-by: Vincent Jardin <vjardin@free.fr> > --- > package/pkg-generic.mk | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk > index 9ec84d0f45..96cc5a1d25 100644 > --- a/package/pkg-generic.mk > +++ b/package/pkg-generic.mk > @@ -176,7 +176,7 @@ $(BUILD_DIR)/%/.stamp_downloaded: > $(call prepare-per-package-directory,$($(PKG)_FINAL_DOWNLOAD_DEPENDENCIES)) > $(foreach hook,$($(PKG)_PRE_DOWNLOAD_HOOKS),$(call $(hook))$(sep)) > # Only show the download message if it isn't already downloaded > - $(Q)for p in $($(PKG)_ALL_DOWNLOADS); do \ > + $(Q)for p in "$($(PKG)_ALL_DOWNLOADS)"; do \ > if test ! -e $($(PKG)_DL_DIR)/`basename $$p` ; then \ > $(call MESSAGE,"Downloading") ; \ > break ; \ > -- > 2.47.0 > > _______________________________________________ > buildroot mailing list > buildroot@buildroot.org > https://lists.buildroot.org/mailman/listinfo/buildroot
Am 20.11.24 um 09:03 schrieb Vincent Jardin: > On Wed, Nov 20, 2024 at 08:37:55AM UTC, Simon Richter via buildroot wrote: >> This allows <PKG>_SOURCE to contain characters like `&`. >> >> Signed-off-by: Simon Richter <simon@sinic.eu> > It is a bad idea to have such name, but it makes sense. I agree. I am trying to download a file from a private file share in Azure DevOps. For authentication, I need to add some data to the URL, separated by '?' or '&': <PKG>_SOURCE = <FILE>?foo&bar&baz&blah If there is a better way to solve this, I will be happy to implement it. Regards, Simon > > LGTM > > Reviewed-by: Vincent Jardin <vjardin@free.fr> >> --- >> package/pkg-generic.mk | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk >> index 9ec84d0f45..96cc5a1d25 100644 >> --- a/package/pkg-generic.mk >> +++ b/package/pkg-generic.mk >> @@ -176,7 +176,7 @@ $(BUILD_DIR)/%/.stamp_downloaded: >> $(call prepare-per-package-directory,$($(PKG)_FINAL_DOWNLOAD_DEPENDENCIES)) >> $(foreach hook,$($(PKG)_PRE_DOWNLOAD_HOOKS),$(call $(hook))$(sep)) >> # Only show the download message if it isn't already downloaded >> - $(Q)for p in $($(PKG)_ALL_DOWNLOADS); do \ >> + $(Q)for p in "$($(PKG)_ALL_DOWNLOADS)"; do \ >> if test ! -e $($(PKG)_DL_DIR)/`basename $$p` ; then \ >> $(call MESSAGE,"Downloading") ; \ >> break ; \ >> -- >> 2.47.0 >> >> _______________________________________________ >> buildroot mailing list >> buildroot@buildroot.org >> https://lists.buildroot.org/mailman/listinfo/buildroot > _______________________________________________ > buildroot mailing list > buildroot@buildroot.org > https://lists.buildroot.org/mailman/listinfo/buildroot
Simon, All, On 2024-11-20 08:37 +0100, Simon Richter via buildroot spake thusly: > This allows <PKG>_SOURCE to contain characters like `&`. > > Signed-off-by: Simon Richter <simon@sinic.eu> > --- > package/pkg-generic.mk | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk > index 9ec84d0f45..96cc5a1d25 100644 > --- a/package/pkg-generic.mk > +++ b/package/pkg-generic.mk > @@ -176,7 +176,7 @@ $(BUILD_DIR)/%/.stamp_downloaded: > $(call prepare-per-package-directory,$($(PKG)_FINAL_DOWNLOAD_DEPENDENCIES)) > $(foreach hook,$($(PKG)_PRE_DOWNLOAD_HOOKS),$(call $(hook))$(sep)) > # Only show the download message if it isn't already downloaded > - $(Q)for p in $($(PKG)_ALL_DOWNLOADS); do \ > + $(Q)for p in "$($(PKG)_ALL_DOWNLOADS)"; do \ What this change is doing is basically make the for-loop useless. $($(PKG)_ALL_DOWNLOADS) is a space-separated list, that may contain more than one item, so it must not be quoted. See for example the cvs package: $ make defconfig $ make show-vars 2>&1 |jq .CVS_ALL_DOWNLOADS { "expanded": "http+http://snapshot.debian.org/archive/debian/20141023T043132Z/pool/main/c/cvs/cvs_1.12.13.orig.tar.gz http+http://snapshot.debian.org/archive/debian/20141023T043132Z/pool/main/c/cvs/cvs_1.12.13-12+squeeze1.diff.gz", "raw": "$(CVS_MAIN_DOWNLOAD) $(CVS_ADDITIONAL_DOWNLOADS)" } So, this is not the proper solution to your issue. Indeed, having `&` in a filename is really asking for trouble all along the way! :-/ As this is a downloaded file, have you tried URL-escaping the `&` with '%26' instead? If it works, that would give a one-off solution for your case, but maybe we need to handle that in a generic way (somehow)... Regards, Yann E. MORIN. > if test ! -e $($(PKG)_DL_DIR)/`basename $$p` ; then \ > $(call MESSAGE,"Downloading") ; \ > break ; \ > -- > 2.47.0 > > _______________________________________________ > buildroot mailing list > buildroot@buildroot.org > https://lists.buildroot.org/mailman/listinfo/buildroot
Hi Simon,
On Wed, Nov 20, 2024 at 09:32:02AM UTC, Simon Richter wrote:
> If there is a better way to solve this, I will be happy to implement it.
$ echo "foo&b a;r" | sed 's/&/%26/g; s/?/%3F/g; s/;/%3B/g; s/ /%20/g'
foo%26b%20a%3Br
best regards,
Vincent
Hi Yann, hi Vincent, Am 20.11.24 um 09:38 schrieb yann.morin@orange.com: > Simon, All, > > On 2024-11-20 08:37 +0100, Simon Richter via buildroot spake thusly: >> This allows <PKG>_SOURCE to contain characters like `&`. >> >> Signed-off-by: Simon Richter<simon@sinic.eu> >> --- >> package/pkg-generic.mk | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk >> index 9ec84d0f45..96cc5a1d25 100644 >> --- a/package/pkg-generic.mk >> +++ b/package/pkg-generic.mk >> @@ -176,7 +176,7 @@ $(BUILD_DIR)/%/.stamp_downloaded: >> $(call prepare-per-package-directory,$($(PKG)_FINAL_DOWNLOAD_DEPENDENCIES)) >> $(foreach hook,$($(PKG)_PRE_DOWNLOAD_HOOKS),$(call $(hook))$(sep)) >> # Only show the download message if it isn't already downloaded >> - $(Q)for p in $($(PKG)_ALL_DOWNLOADS); do \ >> + $(Q)for p in "$($(PKG)_ALL_DOWNLOADS)"; do \ > What this change is doing is basically make the for-loop useless. > > $($(PKG)_ALL_DOWNLOADS) is a space-separated list, that may contain more > than one item, so it must not be quoted. > > See for example the cvs package: > > $ make defconfig > $ make show-vars 2>&1 |jq .CVS_ALL_DOWNLOADS > { > "expanded": "http+http://snapshot.debian.org/archive/debian/20141023T043132Z/pool/main/c/cvs/cvs_1.12.13.orig.tar.gz http+http://snapshot.debian.org/archive/debian/20141023T043132Z/pool/main/c/cvs/cvs_1.12.13-12+squeeze1.diff.gz", > "raw": "$(CVS_MAIN_DOWNLOAD) $(CVS_ADDITIONAL_DOWNLOADS)" > } > > So, this is not the proper solution to your issue. Thanks for the explanation. You're right of course, we can't apply the patch like that, I should have seen that too. > Indeed, having `&` in a filename is really asking for trouble all along > the way! :-/ As this is a downloaded file, have you tried URL-escaping > the `&` with '%26' instead? If it works, that would give a one-off > solution for your case, but maybe we need to handle that in a generic > way (somehow)... I tried to encode the URL (thanks @Vincent), but Azure doesn't like it: 400 The value for one of the HTTP headers is not in the correct format. Our current solution is to abuse <PKG>_DL_OPTS as we don't need the variable (with our packages to be loaded via Azure). We have our own download script (a customized copy of support/download/wget): <while getopts> shift $((OPTIND-1)) sas="$1” shift url=${url#*+*+} # remove the 'azcopy+' before 'https://...' _wget() { echo ${WGET} “${@}” eval ${WGET} “${@}” } _wget -O “‘${output}’” “‘${url}/${filename}${sas}’” In my opinion this is more of a hack than a solution. Do you have any ideas on how we could solve this properly (so that we can bring it upstream)? Thanks and regards, Simon > Regards, > Yann E. MORIN. > >> if test ! -e $($(PKG)_DL_DIR)/`basename $$p` ; then \ >> $(call MESSAGE,"Downloading") ; \ >> break ; \ >> -- >> 2.47.0 >> >> _______________________________________________ >> buildroot mailing list >> buildroot@buildroot.org >> https://lists.buildroot.org/mailman/listinfo/buildroot
diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk index 9ec84d0f45..96cc5a1d25 100644 --- a/package/pkg-generic.mk +++ b/package/pkg-generic.mk @@ -176,7 +176,7 @@ $(BUILD_DIR)/%/.stamp_downloaded: $(call prepare-per-package-directory,$($(PKG)_FINAL_DOWNLOAD_DEPENDENCIES)) $(foreach hook,$($(PKG)_PRE_DOWNLOAD_HOOKS),$(call $(hook))$(sep)) # Only show the download message if it isn't already downloaded - $(Q)for p in $($(PKG)_ALL_DOWNLOADS); do \ + $(Q)for p in "$($(PKG)_ALL_DOWNLOADS)"; do \ if test ! -e $($(PKG)_DL_DIR)/`basename $$p` ; then \ $(call MESSAGE,"Downloading") ; \ break ; \
This allows <PKG>_SOURCE to contain characters like `&`. Signed-off-by: Simon Richter <simon@sinic.eu> --- package/pkg-generic.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)