[Buildroot] [git commit branch/next] DOWNLOAD_WGET: use -O instead of -P to set output file

[Buildroot] [git commit branch/next] DOWNLOAD_WGET: use -O instead of -P to set output file

[Peter Korsgaard]

commit: http://git.buildroot.net/buildroot/commit/?id=cf2486bf317e4bbf88c801fb96183ba62be78cc8
branch: http://git.buildroot.net/buildroot/commit/?id=refs/heads/next

Some URLs use url-encoded arguments to specify the file to download.
Wget will then use an output file name which includes all the ampersands
and such.  However, this is not what we see from buildroot.

E.g.:
FOO_SITE="http://git.foo.com/?p=foo.git;a=blob;f="
FOO_SOURCE="foo.tgz"

wget will download this into $(DL_DIR)/index.html?p=foo.git;a=blob;f=foo.tgz
buildroot thinks it's in $(DL_DIR)/foo.tgz

To make sure the view of wget and buildroot are consistent, specify
the output file explicitly.

Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Signed-off-by: Peter Korsgaard <jacmet@sunsite.dk>
---
 package/Makefile.package.in |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/package/Makefile.package.in b/package/Makefile.package.in
index 33461b4..ea44c6c 100644
--- a/package/Makefile.package.in
+++ b/package/Makefile.package.in
@@ -238,7 +238,7 @@ endef
 
 define DOWNLOAD_WGET
 	test -e $(DL_DIR)/$(2) || \
-	$(WGET) -P $(DL_DIR) $(call qstrip,$(1))/$(2)
+	$(WGET) -O $(DL_DIR)/$(2) $(call qstrip,$(1))/$(2)
 endef
 
 define SOURCE_CHECK_WGET

[Buildroot] [git commit branch/next] DOWNLOAD_WGET: use -O instead of -P to set output file

[Spenser Gilliland]

I'm having an issue with this patch.  The semicolons in the url are
being interpreted literally.  This leads to the following being
downloaded instead of the latter.

--2012-02-14 09:43:24--  http://git.xilinx.com/?p=xldk/microblaze_v2.0.git

vs

--2012-02-14 09:43:24--
http://git.xilinx.com/?p=xldk/microblaze_v2.0.git;a=blob;f=microblze-unknown-linux-gnu.tgz

The toolchain is downloaded using the following lines in
toolchain/toolchain-external/ext-tool.mk.

TOOLCHAIN_EXTERNAL_SITE="http://git.xilinx.com/?p=xldk/microblaze_v2.0.git;a=blob;f="
TOOLCHAIN_EXTERNAL_SOURCE=microblaze-unknown-linux-gnu.tgz

Thanks,
Spenser

On Tue, Feb 14, 2012 at 5:07 AM, Peter Korsgaard <jacmet@sunsite.dk> wrote:
> commit: http://git.buildroot.net/buildroot/commit/?id=cf2486bf317e4bbf88c801fb96183ba62be78cc8
> branch: http://git.buildroot.net/buildroot/commit/?id=refs/heads/next
>
> Some URLs use url-encoded arguments to specify the file to download.
> Wget will then use an output file name which includes all the ampersands
> and such. ?However, this is not what we see from buildroot.
>
> E.g.:
> FOO_SITE="http://git.foo.com/?p=foo.git;a=blob;f="
> FOO_SOURCE="foo.tgz"
>
> wget will download this into $(DL_DIR)/index.html?p=foo.git;a=blob;f=foo.tgz
> buildroot thinks it's in $(DL_DIR)/foo.tgz
>
> To make sure the view of wget and buildroot are consistent, specify
> the output file explicitly.
>
> Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> Signed-off-by: Peter Korsgaard <jacmet@sunsite.dk>
> ---
> ?package/Makefile.package.in | ? ?2 +-
> ?1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/package/Makefile.package.in b/package/Makefile.package.in
> index 33461b4..ea44c6c 100644
> --- a/package/Makefile.package.in
> +++ b/package/Makefile.package.in
> @@ -238,7 +238,7 @@ endef
>
> ?define DOWNLOAD_WGET
> ? ? ? ?test -e $(DL_DIR)/$(2) || \
> - ? ? ? $(WGET) -P $(DL_DIR) $(call qstrip,$(1))/$(2)
> + ? ? ? $(WGET) -O $(DL_DIR)/$(2) $(call qstrip,$(1))/$(2)
> ?endef
>
> ?define SOURCE_CHECK_WGET
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot



-- 
Spenser Gilliland
Computer Engineer
Illinois Institute of Technology

[Buildroot] [git commit branch/next] DOWNLOAD_WGET: use -O instead of -P to set output file

[Alvaro Gamez]

Maybe it's possible to escape them?

TOOLCHAIN_EXTERNAL_SITE=
"http://git.xilinx.com/?p=xldk/microblaze_v2.0.git\;a=blob;f=<http://git.xilinx.com/?p=xldk/microblaze_v2.0.git;a=blob;f=>
"

2012/2/14 Spenser Gilliland <spenser309@gmail.com>

> I'm having an issue with this patch.  The semicolons in the url are
> being interpreted literally.  This leads to the following being
> downloaded instead of the latter.
>
> --2012-02-14 09:43:24--  http://git.xilinx.com/?p=xldk/microblaze_v2.0.git
>
> vs
>
> --2012-02-14 09:43:24--
>
> http://git.xilinx.com/?p=xldk/microblaze_v2.0.git;a=blob;f=microblze-unknown-linux-gnu.tgz
>
> The toolchain is downloaded using the following lines in
> toolchain/toolchain-external/ext-tool.mk.
>
> TOOLCHAIN_EXTERNAL_SITE="
> http://git.xilinx.com/?p=xldk/microblaze_v2.0.git;a=blob;f="
> TOOLCHAIN_EXTERNAL_SOURCE=microblaze-unknown-linux-gnu.tgz
>
> Thanks,
> Spenser
>
> On Tue, Feb 14, 2012 at 5:07 AM, Peter Korsgaard <jacmet@sunsite.dk>
> wrote:
> > commit:
> http://git.buildroot.net/buildroot/commit/?id=cf2486bf317e4bbf88c801fb96183ba62be78cc8
> > branch: http://git.buildroot.net/buildroot/commit/?id=refs/heads/next
> >
> > Some URLs use url-encoded arguments to specify the file to download.
> > Wget will then use an output file name which includes all the ampersands
> > and such.  However, this is not what we see from buildroot.
> >
> > E.g.:
> > FOO_SITE="http://git.foo.com/?p=foo.git;a=blob;f="
> > FOO_SOURCE="foo.tgz"
> >
> > wget will download this into
> $(DL_DIR)/index.html?p=foo.git;a=blob;f=foo.tgz
> > buildroot thinks it's in $(DL_DIR)/foo.tgz
> >
> > To make sure the view of wget and buildroot are consistent, specify
> > the output file explicitly.
> >
> > Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> > Signed-off-by: Peter Korsgaard <jacmet@sunsite.dk>
> > ---
> >  package/Makefile.package.in |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/package/Makefile.package.in b/package/Makefile.package.in
> > index 33461b4..ea44c6c 100644
> > --- a/package/Makefile.package.in
> > +++ b/package/Makefile.package.in
> > @@ -238,7 +238,7 @@ endef
> >
> >  define DOWNLOAD_WGET
> >        test -e $(DL_DIR)/$(2) || \
> > -       $(WGET) -P $(DL_DIR) $(call qstrip,$(1))/$(2)
> > +       $(WGET) -O $(DL_DIR)/$(2) $(call qstrip,$(1))/$(2)
> >  endef
> >
> >  define SOURCE_CHECK_WGET
> > _______________________________________________
> > buildroot mailing list
> > buildroot at busybox.net
> > http://lists.busybox.net/mailman/listinfo/buildroot
>
>
>
> --
> Spenser Gilliland
> Computer Engineer
> Illinois Institute of Technology
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
>



-- 
?lvaro G?mez Machado
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20120214/36861a03/attachment.html>

[Buildroot] [git commit branch/next] DOWNLOAD_WGET: use -O instead of -P to set output file

[Peter Korsgaard]

>>>>> "Alvaro" == Alvaro Gamez <alvaro.gamez@hazent.com> writes:

 Alvaro> Maybe it's possible to escape them?
 Alvaro> TOOLCHAIN_EXTERNAL_SITE="http://git.xilinx.com/?p=xldk/microblaze_v2.0.git\;a=
 Alvaro> blob;f="

Yes, or use single quotes (') instead of double quotes (").

 >> ? ? ? ?test -e $(DL_DIR)/$(2) || \
 >> - ? ? ? $(WGET) -P $(DL_DIR) $(call qstrip,$(1))/$(2)
 >> + ? ? ? $(WGET) -O $(DL_DIR)/$(2) $(call qstrip,$(1))/$(2)

We should probably change it to:

$(WGET) -O $(DL_DIR)/$(2) '$(call qstrip,$(1))/$(2)'

-- 
Bye, Peter Korsgaard

[Buildroot] [git commit branch/next] DOWNLOAD_WGET: use -O instead of -P to set output file

[Spenser Gilliland]

This workaround works; however, the default is to prepend a / to the
front of the source name.  This leads to the following being
downloaded.

--2012-02-14 10:10:40--
http://git.xilinx.com/?p=xldk/microblaze_v2.0.git;a=blob;f=/microblaze-unknown-linux-gnu.tgz

instead of

--2012-02-14 10:10:40--
http://git.xilinx.com/?p=xldk/microblaze_v2.0.git;a=blob;f=microblaze-unknown-linux-gnu.tgz

Without the slash the download should work. Peter should the patch
remove the prepended slash as well?

Spenser

On Tue, Feb 14, 2012 at 10:01 AM, Peter Korsgaard <jacmet@sunsite.dk> wrote:
>>>>>> "Alvaro" == Alvaro Gamez <alvaro.gamez@hazent.com> writes:
>
> ?Alvaro> Maybe it's possible to escape them?
> ?Alvaro> TOOLCHAIN_EXTERNAL_SITE="http://git.xilinx.com/?p=xldk/microblaze_v2.0.git\;a=
> ?Alvaro> blob;f="
>
> Yes, or use single quotes (') instead of double quotes (").
>
> ?>> ? ? ? ?test -e $(DL_DIR)/$(2) || \
> ?>> - ? ? ? $(WGET) -P $(DL_DIR) $(call qstrip,$(1))/$(2)
> ?>> + ? ? ? $(WGET) -O $(DL_DIR)/$(2) $(call qstrip,$(1))/$(2)
>
> We should probably change it to:
>
> $(WGET) -O $(DL_DIR)/$(2) '$(call qstrip,$(1))/$(2)'
>
> --
> Bye, Peter Korsgaard



-- 
Spenser Gilliland
Computer Engineer
Illinois Institute of Technology

[Buildroot] [git commit branch/next] DOWNLOAD_WGET: use -O instead of -P to set output file

[Peter Korsgaard]

>>>>> "Spenser" == Spenser Gilliland <spenser309@gmail.com> writes:

 Spenser> This workaround works; however, the default is to prepend a / to the
 Spenser> front of the source name.  This leads to the following being
 Spenser> downloaded.

 Spenser> Without the slash the download should work. Peter should the patch
 Spenser> remove the prepended slash as well?

I guess so. We would need to audit that all <blah>_SITE have a trailing
space though, and this could potentially break out-of-tree stuff.

-- 
Bye, Peter Korsgaard

[Buildroot] [git commit branch/next] DOWNLOAD_WGET: use -O instead of -P to set output file

[Spenser Gilliland]

Peter,

After greping through the source the enormity of that patch and the
potential for out of tree breakage suggest a different course of
action.  A possible solution may be to scan the SITE var for a
question mark and only change to non-slash mode on that occasion.
Would this be reasonable?

Spenser

On Tue, Feb 14, 2012 at 10:30 AM, Peter Korsgaard <jacmet@sunsite.dk> wrote:
>>>>>> "Spenser" == Spenser Gilliland <spenser309@gmail.com> writes:
>
> ?Spenser> This workaround works; however, the default is to prepend a / to the
> ?Spenser> front of the source name. ?This leads to the following being
> ?Spenser> downloaded.
>
> ?Spenser> Without the slash the download should work. Peter should the patch
> ?Spenser> remove the prepended slash as well?
>
> I guess so. We would need to audit that all <blah>_SITE have a trailing
> space though, and this could potentially break out-of-tree stuff.
>
> --
> Bye, Peter Korsgaard



-- 
Spenser Gilliland
Computer Engineer
Illinois Institute of Technology

[Buildroot] [git commit branch/next] DOWNLOAD_WGET: use -O instead of -P to set output file

[Peter Korsgaard]

>>>>> "Spenser" == Spenser Gilliland <spenser309@gmail.com> writes:

 Spenser> Peter,
 Spenser> After greping through the source the enormity of that patch and the
 Spenser> potential for out of tree breakage suggest a different course of
 Spenser> action.  A possible solution may be to scan the SITE var for a
 Spenser> question mark and only change to non-slash mode on that occasion.
 Spenser> Would this be reasonable?

Hmm, neither solution is really nice. Let me think a bit more about it.

-- 
Bye, Peter Korsgaard

[Buildroot] [git commit branch/next] DOWNLOAD_WGET: use -O instead of -P to set output file

[Arnout Vandecappelle]

On Tuesday 14 February 2012 18:54:22 Spenser Gilliland wrote:
> After greping through the source the enormity of that patch and the
> potential for out of tree breakage suggest a different course of
> action.  A possible solution may be to scan the SITE var for a
> question mark and only change to non-slash mode on that occasion.
> Would this be reasonable?

 Yeah, my patch was too easy :-)

 I think it's better to keep the split at the /

 We could use either
microblaze_v2.0.git;a=blob;f=microblaze-unknown-linux-gnu.tgz
or
?p=xldk/microblaze_v2.0.git;a=blob;f=microblaze-unknown-linux-gnu.tgz
as the _SOURCE.  Of course, that still requires some serious massaging
to avoid funky filenames that break everything.  So probably not an
option either.

 I think on the short term it is probably best to use a work-around
like for the blackfin toolchain: a non-standard download target, that
doesn't use the DOWNLOAD function but a custom $(WGET) call.

 On the longer term we could try to refactor that into the generic
DOWNLOAD function.

 Regards,
 Arnout

-- 
Arnout Vandecappelle                               arnout at mind be
Senior Embedded Software Architect                 +32-16-286540
Essensium/Mind                                     http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium                BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20120214/dc64ecd5/attachment-0001.html>