pkg/39208: Using curl for FETCH_CMD is broken

[brian.de.alwis%usask.ca@localhost]

>Number:         39208
>Category:       pkg
>Synopsis:       Using curl for FETCH_CMD is broken
>Confidential:   no
>Severity:       serious
>Priority:       high
>Responsible:    pkg-manager
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri Jul 25 17:40:00 +0000 2008
>Originator:     Brian de Alwis
>Release:        pkgsrc-current
>Organization:
University of Saskatchewan
>Environment:
Darwin beast 9.4.0 Darwin Kernel Version 9.4.0: Mon Jun  9 19:30:53 PDT 2008; 
root:xnu-1228.5.20~1/RELEASE_I386 i386
>Description:
I posted about this to tech-pkg a while back, but forgot to follow-up wth a PR 
(Message-Id < DA8F765B-0FF5-43F1-9E8D-461C647880BC%usask.ca@localhost>).

I have been trying to use FETCH_CMD=curl as curl comes with a default set of 
certificates and thus a good candidate for fetching files using https.  But 
I've encountered a few problems and in the process of trying to get it to work, 
I've found the description of FETCH_OUTPUT_ARGS to be unclear, and also found 
that the example using wget in the pkgsrc guide doesn't actually work.

curl, by default, sends its output to stdout.  curl can be directed to save the 
content to an explicitly specified file using '-o filename'.  mk/fetch/fetch 
describes FETCH_OUTPUT_ARGS as:

#       FETCH_OUTPUT_ARGS
#                       These options specify the name of the local file
#                       that will hold the contents of the fetched file.

This wording is a bit unclear -- the variable doesn't specify the local file, 
but provides the arguments necessary to specify the output file. From looking 
at the example in the pkgsrc guide and the mk/fetch/fetch code, I think this is 
better worded as:

# FETCH_OUTPUT_ARGS
#       These options are used to specify the name of the local file
#       that will hold the contents of the fetched file.

FETCH_OUTPUT_ARGS doesn't actually seem to be used except for resumed fetches 
(which is not the default).  mk/fetch/fetch does the fetch using:

         if ${TEST} -n "$resume"; then
               fetch_cmd="${FETCH_CMD} ${FETCH_BEFORE_ARGS} 
${FETCH_RESUME_ARGS} ${FETCH_OUTPUT_ARGS} $outputfile $site$file"
         else
               fetch_cmd="${FETCH_CMD} ${FETCH_BEFORE_ARGS} $site$file 
${FETCH_AFTER_ARGS}"
         fi

I think it makes more sense to specify `${FETCH_OUTPUT_ARGS} $outputfile' for 
both cases (patch appended below).  After all, the two commands should be 
mirrors except for the FETCH_RESUME_ARGS, right?

With this change, I can use curl to fetch files using the following:

FETCH_CMD=              curl
FETCH_BEFORE_ARGS=
FETCH_OUTPUT_ARGS=      -o
FETCH_RESUME_ARGS=

Without the patch, curl cannot be used as a fetch command.


>How-To-Repeat:
Install net/curl

$ cd x11/gtk2
$ rm ../../distfiles/gtk+-2*
$ make FETCH_CMD=curl FETCH_BEFORE_ARGS="" FETCH_OUTPUT_ARGS=-o 
FETCH_RESUME_ARGS="" fetch

see the file output go to the console rather than to a file
>Fix:
Only in /scratch/dev/cvs-pkgsrc.org/mk/fetch/: CVS
diff -ruwb /scratch/dev/cvs-pkgsrc.org/mk/fetch/fetch mk/fetch/fetch
--- /scratch/dev/cvs-pkgsrc.org/mk/fetch/fetch  2007-08-04 19:49:47.000000000 
-0600
+++ mk/fetch/fetch      2008-02-29 18:36:32.000000000 -0600
@@ -99,8 +99,9 @@
 #                      previous file transfer.
 #
 #      FETCH_OUTPUT_ARGS
-#                      These options specify the name of the local file
-#                      that will hold the contents of the fetched file.
+#                      These options are used to specify the name of
+#                      the local file that will hold the contents of the
+#                      fetched file.
 #
 ######################################################################
 
@@ -262,7 +263,7 @@
          if ${TEST} -n "$resume"; then
                fetch_cmd="${FETCH_CMD} ${FETCH_BEFORE_ARGS} 
${FETCH_RESUME_ARGS} ${FETCH_OUTPUT_ARGS} $outputfile $site$file"
          else
-               fetch_cmd="${FETCH_CMD} ${FETCH_BEFORE_ARGS} $site$file 
${FETCH_AFTER_ARGS}"
+               fetch_cmd="${FETCH_CMD} ${FETCH_BEFORE_ARGS} 
${FETCH_OUTPUT_ARGS} $outputfile $site$file ${FETCH_AFTER_ARGS}"
          fi
          ${TEST} -z "$verbose" || ${ECHO} "$fetch_cmd"
          $fetch_cmd )