Re: opinion needed on feature/download
Sorry, surely I've not sent it ? anyway, continuing.. On 28 November 2012 09:28, Matúš Kukan wrote: > On 27 November 2012 23:56, Mat M wrote: >>> It's the other way around, they will be downloaded even if we don't >>> necessarily need them. >>> See BUILD_TYPE="$BUILD_TYPE MOZ" above in configure. >>> If PREBUILD_MOZAB == YES then also MOZ is in BUILD_TYPE. >> >> >> So it is a regression from before where we got them only if we build it. It >> is ~10 Mb, which could be saved from bandwith, IMO. Even if not so large >> compared to the whole set of tarballs. >> Maybe I am too tied to this code I tweaked lately :) > Sure, but for who ? I believe we build moz/ only for Windows ? where test "$MSVSVER" != "2005" is true, so PREBUILD_MOZAB must be YES ? Anyway, if I am wrong or you just insist, change: $(call fetch_Optional,MOZ, -> $(if $(filter YES,$(PREBUILD_MOZAB)), in Makefile.fetch Best, Matus ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: opinion needed on feature/download
On 27 November 2012 23:56, Mat M wrote: >> It's the other way around, they will be downloaded even if we don't >> necessarily need them. >> See BUILD_TYPE="$BUILD_TYPE MOZ" above in configure. >> If PREBUILD_MOZAB == YES then also MOZ is in BUILD_TYPE. > > > So it is a regression from before where we got them only if we build it. It > is ~10 Mb, which could be saved from bandwith, IMO. Even if not so large > compared to the whole set of tarballs. > Maybe I am too tied to this code I tweaked lately :) Sure, but for who ? I believe we build moz/ only for Windows where ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: opinion needed on feature/download
Hello, that's me again. Le Tue, 27 Nov 2012 11:44:08 +0100, Matúš Kukan a écrit: On 27 November 2012 02:18, Mat M wrote: Ah. yes, but without setting the BUILDTYPE with a pattern that matches the tar name, how do you trigger the download ? * the pattern is not important, we don't do any magic and use it always explicitly, so: GRAPHITE:graphite in prj/build.lst or $(call fetch_Optional,GRAPHITE,$(GRAPHITE_TARBALL)) in Makefile.fetch If BUILD_TYPE does not include GRAPHITE, GRAPHITE_TARBALL is not retrieved. But I admit I reverted the process by saying "- Add a block to test for the need of the tarball in configure.ac", because if you want your module built, then you set the variable, and as a consequence, you get the tarball. But their names now must be defined in download.lst. For mozilla stuff I've used MOZ, for libxmlsec you could use DESKTOP but it will be needed for build platform anyway (and downloaded) Sure but you did not set BUILD_TYPE next to PREBUILD_MOZAB="YES", so they won't be downloaded, IMO (somewhat related to my previous question, btw) It's the other way around, they will be downloaded even if we don't necessarily need them. See BUILD_TYPE="$BUILD_TYPE MOZ" above in configure. If PREBUILD_MOZAB == YES then also MOZ is in BUILD_TYPE. So it is a regression from before where we got them only if we build it. It is ~10 Mb, which could be saved from bandwith, IMO. Even if not so large compared to the whole set of tarballs. Maybe I am too tied to this code I tweaked lately :) Rant ended with a sigh :)) -- Mat M ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: opinion needed on feature/download
Hi, On Tue, Nov 27, 2012 at 02:01:59PM +0100, Petr Mladek wrote: > Matúš Kukan píše v Út 27. 11. 2012 v 13:34 +0100: > > On 26 November 2012 11:46, Petr Mladek wrote: > > > It was me who come up with the idea or removing md5sum from the file > > > name. Well, it was after Matus came up with the idea of introducing .md5 > > > files :-) > > > > Wasn't it David who came up with .md5 files ? > > https://bugs.freedesktop.org/show_bug.cgi?id=56131#c1 > > But I think he meant to have them in git, not on web. > > I see. The .md5 and even .url file was David's idea and I was not sure > about the real meaning. Some of the things I wanted to achieve were: * make it easy to get all tarballs (sed -e 's/\s\+/-/' */*.md5) * avoid the profusion of variables only used in one place * make it easy to update the tarball (md5sum foo-x.y.tar.gz > foo.md5) * while it be still possible to download just the used tarballs (for both sides: host and build) I have not thought much about the way to get the list of used tarballs from gbuild. We could, e.g., create a makefile that redefines gb_UnpackedTarball_set_tarball (which would now take the md5 file instead of tarball) to collect tarballs into a variable, loads all external modules and then prints that variable. The .url files were purely to allow the current practice of having the tarballs in more than one place. And yes, both .md5 and .url would be in git. > > > As I said, I do not have any strong opinion. If we remove md5sum from > > > the filename, it will cause extra work for distro package maintainers. > > > On the other hand, it might be slightly easier in the long term. > > > > Me neither. > > So, probably does not make sense to discus this unless someone decides > > we want to change it. I am for getting rid of the md5 in the tarballs' names. I always thought there was some reason behind it... This also makes me reluctant to advance the abovementioned scheme, because it feels like overkill. But hey, it was just an idea :-) D. ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: opinion needed on feature/download
On Tue, Nov 27, 2012 at 7:01 AM, Petr Mladek wrote: > > Or we want to make sure that people use the only single version of the > tarballs (security?, preciseness?). In this case, we might need md5sum > in git. But this is pretty non-standart solution. I think that it is too > paranoid and I am not sure if it is worth the effort having the complex > names. For example, if you want to work with the file and do not > remember md5sum, you need to search the directory to be able to write > the right name... I do think that it is worth the effort. we do point to tarball that are not hosted directly, and we do want to detect an intrusion. If someone hack our infra and mess with the git repo... since 100's of people have a copy of the git repo we will notice a hack there... but if the md5 value is not in git itself then someone that hack the server can inject his own tarball and that would no be detected unless someone cafefully inspect the tarball or get a md5 independently of the original tarball... so there is no real point of using md5 if we are not keeping the 'value' in git itself (and no the dowload integrity check _is_ not worth it... if a download fail you usually know, and even if you do not, that rarely result in something that you can uncompress and untar without error Norbert ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: opinion needed on feature/download
Matúš Kukan píše v Út 27. 11. 2012 v 13:34 +0100: > On 26 November 2012 11:46, Petr Mladek wrote: > > It was me who come up with the idea or removing md5sum from the file > > name. Well, it was after Matus came up with the idea of introducing .md5 > > files :-) > > Wasn't it David who came up with .md5 files ? > https://bugs.freedesktop.org/show_bug.cgi?id=56131#c1 > But I think he meant to have them in git, not on web. I see. The .md5 and even .url file was David's idea and I was not sure about the real meaning. > > IMHO, you need not have md5 in git to detect the download corruption. If > > a file is corrupted, the md5 sum from the .md5 file does not match. Or > > did I miss something? > > But then you would also need to check md5sum of the .md5 file, that's > recursive problem. > They need to be in git, otherwise there is no point in having them. > md5sum could match if they were created that way and passed to you somehow. > (Unless, I am missing something.) The question is what is the purpose of the md5sum here: We either want to check that the file was not corrupted during the download (network failure). In this case, the .md5 file is enough and we do not need to have it git. Note that there is no recursion. If the .md5 file is corrupted, md5sum does not match and you just need to download both files again. Or we want to make sure that people use the only single version of the tarballs (security?, preciseness?). In this case, we might need md5sum in git. But this is pretty non-standart solution. I think that it is too paranoid and I am not sure if it is worth the effort having the complex names. For example, if you want to work with the file and do not remember md5sum, you need to search the directory to be able to write the right name... > > As I said, I do not have any strong opinion. If we remove md5sum from > > the filename, it will cause extra work for distro package maintainers. > > On the other hand, it might be slightly easier in the long term. > > Me neither. > So, probably does not make sense to discus this unless someone decides > we want to change it. Yup. Best Regards, Petr ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: opinion needed on feature/download
On 26 November 2012 11:46, Petr Mladek wrote: > It was me who come up with the idea or removing md5sum from the file > name. Well, it was after Matus came up with the idea of introducing .md5 > files :-) Wasn't it David who came up with .md5 files ? https://bugs.freedesktop.org/show_bug.cgi?id=56131#c1 But I think he meant to have them in git, not on web. > IMHO, you need not have md5 in git to detect the download corruption. If > a file is corrupted, the md5 sum from the .md5 file does not match. Or > did I miss something? But then you would also need to check md5sum of the .md5 file, that's recursive problem. They need to be in git, otherwise there is no point in having them. md5sum could match if they were created that way and passed to you somehow. (Unless, I am missing something.) > As I said, I do not have any strong opinion. If we remove md5sum from > the filename, it will cause extra work for distro package maintainers. > On the other hand, it might be slightly easier in the long term. Me neither. So, probably does not make sense to discus this unless someone decides we want to change it. All the best, Matus ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: opinion needed on feature/download
On 27 November 2012 02:18, Mat M wrote: > Ah. yes, but without setting the BUILDTYPE with a pattern that matches the > tar name, how do you trigger the download ? * the pattern is not important, we don't do any magic and use it always explicitly, so: GRAPHITE:graphite in prj/build.lst or $(call fetch_Optional,GRAPHITE,$(GRAPHITE_TARBALL)) in Makefile.fetch * the download is triggered in Makefile.fetch, it does not have to be optional, see LIBXMLSEC_TARBALL >> But their names now must be defined in download.lst. >> For mozilla stuff I've used MOZ, for libxmlsec you could use DESKTOP >> but it will be needed for build platform anyway (and downloaded) > > > Sure but you did not set BUILD_TYPE next to PREBUILD_MOZAB="YES", so they > won't be downloaded, IMO (somewhat related to my previous question, btw) It's the other way around, they will be downloaded even if we don't necessarily need them. See BUILD_TYPE="$BUILD_TYPE MOZ" above in configure. If PREBUILD_MOZAB == YES then also MOZ is in BUILD_TYPE. Best, Matus ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: opinion needed on feature/download
Matus, Le Mon, 26 Nov 2012 15:32:51 +0100, Matúš Kukan a écrit: You could have.. I just wanted to let people know here what I am working on to be sure nobody will be strongly against (maybe it was pointless) The more the better :-) - 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 Not really. We use BUILD_TYPE to determine which modules to build (although this information is often redundant with SYSTEM_FOO) Ah. yes, but without setting the BUILDTYPE with a pattern that matches the tar name, how do you trigger the download ? But their names now must be defined in download.lst. For mozilla stuff I've used MOZ, for libxmlsec you could use DESKTOP but it will be needed for build platform anyway (and downloaded) Sure but you did not set BUILD_TYPE next to PREBUILD_MOZAB="YES", so they won't be downloaded, IMO (somewhat related to my previous question, btw) Improvement: Since you check for wget or curl in configure, I would set one variable This would make configure more complicated, I did not want to do that. IMHO configure is just for very basic stuff and exporting variable name for command sounds fine I think. Moreover, I missed that the commandlines are completely different and the gain is lost here. FETCHTAR that will be used in fetch_Download_get_command instead of having if'ed define (BTW, why 2 underscores in function name ?) 2 underscored are just a habit from gbuild. We use __<'public'-method> or ___<'private'-method> defines in gbuild. TY ! Nice to write somewhere for the next generation :) Example gb_UnoApiTarget_add_idlfile vs. gb_UnoApiTarget__add_idlfile in solenv/gbuild/UnoApiTarget.mk Thanks for your careful review, it's really cool. Thanks for the detailed reply. -- Mat M ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: opinion needed on feature/download
Hello Mat On 23 November 2012 19:06, Mat M wrote: > Hello Matus, > > I plannned to answer in the issue, but I saw your comment about the ML,so... You could have.. I just wanted to let people know here what I am working on to be sure nobody will be strongly against (maybe it was pointless) I think it's ~done now and I will probably just push it. (with reverted last commit) > 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 that's right > - 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 Not really. We use BUILD_TYPE to determine which modules to build (although this information is often redundant with SYSTEM_FOO) > What i do not like: > - "too much" machinery in makefile.fetch Indeed, but there is nothing to do about it I am afraid. > 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. But their names now must be defined in download.lst. For mozilla stuff I've used MOZ, for libxmlsec you could use DESKTOP but it will be needed for build platform anyway (and downloaded) > 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. Well, but Makefile.fetch is the first (second) place to look into if you are trying to answer "why-it-does-not-get-downloaded" question, so it should be fine ? > Improvement: > Since you check for wget or curl in configure, I would set one variable This would make configure more complicated, I did not want to do that. IMHO configure is just for very basic stuff and exporting variable name for command sounds fine I think. > FETCHTAR that will be used in fetch_Download_get_command instead of having > if'ed define (BTW, why 2 underscores in function name ?) 2 underscored are just a habit from gbuild. We use __<'public'-method> or ___<'private'-method> defines in gbuild. Example gb_UnoApiTarget_add_idlfile vs. gb_UnoApiTarget__add_idlfile in solenv/gbuild/UnoApiTarget.mk Thanks for your careful review, it's really cool. All the best, Matus ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: opinion needed on feature/download
Mat M píše v Pá 23. 11. 2012 v 19:06 +0100: > Alternative: if someone insists on separating md5 from filename to be able > to use default tars, It was me who come up with the idea or removing md5sum from the file name. Well, it was after Matus came up with the idea of introducing .md5 files :-) I do not have any strong opinion on it. I just think that the md5 sum in the tarball name is quite non-standard solution. I think that the only "advantage" is that the md5sum is hardcoded in git => you are 100% sure that you use the right tarball from the LO site. But most people are using "make fetch", so they are downloading the preferred tarball anyway. > I propose to have a fetch folder with the list of md5 > files like : foobar-1.1.4.tar.gz.md5 containing > 48d647fbd8ef8889e5a7f422c1bfda94 I am not sure what you mean with fetch folder. IMHO, we do not need any extra list anywhere. The md5 file name would be: .md5 The content would be the standard content produced by md5sum utility: , so that you could simply call: md5sum -c .md5 It would be located on the server like the tarball => the same URL prefix. > I prefer having md5 under git, because if md5 is corrupt by the download, > you cannot check your tar download. IMHO, you need not have md5 in git to detect the download corruption. If a file is corrupted, the md5 sum from the .md5 file does not match. Or did I miss something? As I said, I do not have any strong opinion. If we remove md5sum from the filename, it will cause extra work for distro package maintainers. On the other hand, it might be slightly easier in the long term. Best Regards, Petr ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
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 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 ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
