[PATCH] fetch/wget: Start to clean up command construction

[PATCH] fetch/wget: Start to clean up command construction

[Richard Purdie]

Start to clean up wget fetcher command construction to allow clearer
and more extensible code structure. Drops support for ${URI} and
${FILE} directly in the commands.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
diff --git a/bitbake/lib/bb/fetch2/wget.py b/bitbake/lib/bb/fetch2/wget.py
index e2f99ce..bb686b5 100644
--- a/bitbake/lib/bb/fetch2/wget.py
+++ b/bitbake/lib/bb/fetch2/wget.py
@@ -58,28 +58,27 @@ class Wget(FetchMethod):
 
         ud.localfile = data.expand(urllib.unquote(ud.basename), d)
 
+        self.basecmd = d.getVar("FETCHCMD_wget", True) or "/usr/bin/env wget -t 2 -T 30 -nv --passive-ftp --no-check-certificate"
+
     def download(self, ud, d, checkonly = False):
         """Fetch urls"""
 
-        basecmd = d.getVar("FETCHCMD_wget", True) or "/usr/bin/env wget -t 2 -T 30 -nv --passive-ftp --no-check-certificate"
+        fetchcmd = self.basecmd
 
         if not checkonly and 'downloadfilename' in ud.parm:
             dldir = d.getVar("DL_DIR", True)
             bb.utils.mkdirhier(os.path.dirname(dldir + os.sep + ud.localfile))
-            basecmd += " -O " + dldir + os.sep + ud.localfile
+            fetchcmd += " -O " + dldir + os.sep + ud.localfile
 
+        uri = ud.url.split(";")[0]
         if checkonly:
-            fetchcmd = d.expand(basecmd + " --spider '${URI}'")
+            fetchcmd = self.basecmd + " --spider '%s'" % uri
         elif os.path.exists(ud.localpath):
             # file exists, but we didnt complete it.. trying again..
-            fetchcmd = d.expand(basecmd + " -c -P ${DL_DIR} '${URI}'")
+            fetchcmd = self.basecmd + d.expand(" -c -P ${DL_DIR} '%s'" % uri)
         else:
-            fetchcmd = d.expand(basecmd + " -P ${DL_DIR} '${URI}'")
-
-        uri = ud.url.split(";")[0]
+            fetchcmd = self.basecmd + d.expand(" -P ${DL_DIR} '%s'" % uri)
 
-        fetchcmd = fetchcmd.replace("${URI}", uri.split(";")[0])
-        fetchcmd = fetchcmd.replace("${FILE}", ud.basename)
         if not checkonly:
             logger.info("fetch " + uri)
             logger.debug(2, "executing " + fetchcmd)

Re: [PATCH] fetch/wget: Start to clean up command construction

[Kristof Robot]

>Start to clean up wget fetcher command construction to allow clearer
>and more extensible code structure. Drops support for ${URI} and
>${FILE} directly in the commands.
>
>Signed-off-by: Richard Purdie <richard.purdie at linuxfoundation.org>
>[...]
>         if not checkonly and 'downloadfilename' in ud.parm:
>             dldir = d.getVar("DL_DIR", True)
>             bb.utils.mkdirhier(os.path.dirname(dldir + os.sep + ud.localfile))
>-            basecmd += " -O " + dldir + os.sep + ud.localfile
>+            fetchcmd += " -O " + dldir + os.sep + ud.localfile
>
>+        uri = ud.url.split(";")[0]
>         if checkonly:
>-            fetchcmd = d.expand(basecmd + " --spider '${URI}'")
>+            fetchcmd = self.basecmd + " --spider '%s'" % uri
>         elif os.path.exists(ud.localpath):
>             # file exists, but we didnt complete it.. trying again..
>-            fetchcmd = d.expand(basecmd + " -c -P ${DL_DIR} '${URI}'")
>+            fetchcmd = self.basecmd + d.expand(" -c -P ${DL_DIR} '%s'" % uri)
>         else:
>-            fetchcmd = d.expand(basecmd + " -P ${DL_DIR} '${URI}'")
>-

This patch breaks the "downloadfilename" functionality, as the
fetchcmd append in the first if structure will be overwritten by the
fetchcmd assignments in the second if structure, effectively losing
the appended information.

As a quick fix, I replaced

 fetchcmd += " -O " + dldir + os.sep + ud.localfile

by

 self.basecmd += " -O " + dldir + os.sep + ud.localfile

but there might be a better way of resolving this?

Kristof

Re: [PATCH] fetch/wget: Start to clean up command construction

[Richard Purdie]

On Thu, 2014-03-06 at 21:41 +0100, Kristof Robot wrote:
> >Start to clean up wget fetcher command construction to allow clearer
> >and more extensible code structure. Drops support for ${URI} and
> >${FILE} directly in the commands.
> >
> >Signed-off-by: Richard Purdie <richard.purdie at linuxfoundation.org>
> >[...]
> >         if not checkonly and 'downloadfilename' in ud.parm:
> >             dldir = d.getVar("DL_DIR", True)
> >             bb.utils.mkdirhier(os.path.dirname(dldir + os.sep + ud.localfile))
> >-            basecmd += " -O " + dldir + os.sep + ud.localfile
> >+            fetchcmd += " -O " + dldir + os.sep + ud.localfile
> >
> >+        uri = ud.url.split(";")[0]
> >         if checkonly:
> >-            fetchcmd = d.expand(basecmd + " --spider '${URI}'")
> >+            fetchcmd = self.basecmd + " --spider '%s'" % uri
> >         elif os.path.exists(ud.localpath):
> >             # file exists, but we didnt complete it.. trying again..
> >-            fetchcmd = d.expand(basecmd + " -c -P ${DL_DIR} '${URI}'")
> >+            fetchcmd = self.basecmd + d.expand(" -c -P ${DL_DIR} '%s'" % uri)
> >         else:
> >-            fetchcmd = d.expand(basecmd + " -P ${DL_DIR} '${URI}'")
> >-
> 
> This patch breaks the "downloadfilename" functionality, as the
> fetchcmd append in the first if structure will be overwritten by the
> fetchcmd assignments in the second if structure, effectively losing
> the appended information.
> 
> As a quick fix, I replaced
> 
>  fetchcmd += " -O " + dldir + os.sep + ud.localfile
> 
> by
> 
>  self.basecmd += " -O " + dldir + os.sep + ud.localfile
> 
> but there might be a better way of resolving this?

Thanks, looking at the code I think I was intending it to work
differently but confused things somewhere through the various patches.
I've pushed a fix that should resolve this, thanks for the report!

Cheers,

Richard