Re: opinion needed on feature/download

Hello Matus,

I plannned to answer in the issue, but I saw your comment about the  
ML,so...

Le Fri, 23 Nov 2012 13:19:52 +0100, Matúš Kukan <matus.kukan@gmail.com> a  
écrit:
> Hi,
>
> I've made some changes around 'make fetch', ./download, ./ooo.lst in
> feature/download.
> Files are downloaded by makefile now and it is supposed to work better.
> There are still few issues though and I am not sure whether to work on
> them or it's going to be thrown away because it's ugly.
> So, any feedback appreciated.

So now, with your current implementation:
For one tarball foobar-1.1.4.tar.gz we do:
- Specify FOOBAR_TARBALL :=  
"48d647fbd8ef8889e5a7f422c1bfda94-foobar-1.1.4.tar.gz" in download.lst
- Specify $(call fetch_Optional,FOOBAR,$(FOOBAR_TARBALL)) \ to depending  
url call in Makefile.fetch
- Add a block to test for the need of the tarball in configure.ac
if test "$need_foobar" = "yes"; then
    BUILD_TYPE="$BUILD_TYPE FOOBAR"
fi

What i like:
- all tarball names / id are consolidated in one place
- no more machinery in .lst file
- less variables in config_host

What i do not like:
- "too much" machinery in makefile.fetch : You moved in commit  
f9557d7f82648febe40067fa69e96525d938c16b [1] two conditionals from  
configure.ac into it (mozab & libxmlsec). They should go back and TARBALL  
variables replaced with BUILD_TYPE, IMO. If we put some logic in make  
fetch we are doomed to forget/ not know it and kept trapped into  
why-it-does-not-download-although-my-configure-is-ok.

Improvement:
Since you check for wget or curl in configure, I would set one variable  
FETCHTAR that will be used in fetch_Download_get_command instead of having  
if'ed define (BTW, why 2 underscores in function name ?)

Alternative: if someone insists on separating md5 from filename to be able  
to use default tars, I propose to have a fetch folder with the list of md5  
files like : foobar-1.1.4.tar.gz.md5 containing  
48d647fbd8ef8889e5a7f422c1bfda94
I prefer having md5 under git, because if md5 is corrupt by the download,  
you cannot check your tar download.

[1] :  
http://cgit.freedesktop.org/libreoffice/core/diff/configure.ac?id=f9557d7f82648febe40067fa69e96525d938c16b

HTH
Best,
--
Mat M