Re: [PATCH] gbuildification of rhino
Hi, On Wed, Oct 03, 2012 at 01:23:41PM +0200, David Tardon wrote: > Hi, > > On Wed, Oct 03, 2012 at 11:37:17AM +0200, David Ostrovsky wrote: > > Hi Norbert, > > > > On 03.10.2012 01:09, Norbert Thiebaud wrote: > > >David, > > > > > >to answer your question on IRC. > > > > > >yes the default patchlevel for gbuild is 3, because most fo the patch > > >we had are like that... > > really? > > > > Currently, in dmake build system all patches are applied with patch level 2. > > The wrong default patch level (3) in gbuild UnpackedTarball.mk > > gb_UnpackedTarball_PATCHLEVEL_DEFAULT := 3 > > wasn't show up until rhino gbuildification, because in rhino there are two > > build.xml files in different locations. With wrong patch level 3 the > > wrong build.xml > > was picked up and rejected. > > > > grep patch in solenv/inc give that: > > tg_ext.mk:[...] patch $(PATCHFLAGS) -p2 [...] > > Yes, but notice from _where_ the patch is applied. In gbuild it is from > the project's unpacked dir, while in dmake it is one level up. Therefore > in gbuild we have to strip one level more. > > > > > To fix that in gbuild would be as easy as: > > gb_UnpackedTarball_PATCHLEVEL_DEFAULT := 2 > > Which would break apache-commons, beanshell and all other modules > converted up to now that have old-style patches in them .-) That is actually not true I have always thought that patch used the new file (the '+++' line). Well, I have been wrong :-) The reality is more complicated; the selection is described in patch(1) as follows (shortened): "First, patch takes an ordered list of candidate file names as follows: · If the header is that of a context diff, patch takes the old and new file names in the header. A name is ignored if it does not have enough slashes to satisfy the -pnum or --strip=num option. The name /dev/null is also ignored. [...] · For the purpose of the following rules, the candidate file names are considered to be in the order (old, new, index), regardless of the order that they appear in the header. Then patch selects a file name from the candidate list as follows: · If some of the named files exist, patch selects the first name if conforming to POSIX, and the best name otherwise. [...] To determine the best of a nonempty list of file names, patch first takes all the names with the fewest path name components; of those, it then takes all the names with the shortest basename; of those, it then takes all the shortest names; finally, it takes the first remaining name." The problematic hunk in this case is --- misc/rhino1_5R5/toolsrc/build.xml 2004-03-25 21:54:34.0 +0100 +++ misc/build/rhino1_5R5/toolsrc/build.xml 2009-01-17 20:46:44.0 +0100 The situation would be as follows: 1) dmake: - uses -p2 - patch is applied from a dir above rhino1_5R5 patch considers toolsrc/build.xml and rhino1_5R5/toolsrc/build.xml ; only the later exists and is selected. 2. gbuild: - uses -p3 - patch is applied from inside rhino1_5R5 patch considers build.xml and toolsrc/build.xml; both of them exist (because there is top-level build.xml), the former has fewer name components and is selected. BAANG! I am changing the default patch level from 3 to 2 on this evidence. D. ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [PATCH] gbuildification of rhino
Hi, On Wed, Oct 03, 2012 at 11:37:17AM +0200, David Ostrovsky wrote: > Hi Norbert, > > On 03.10.2012 01:09, Norbert Thiebaud wrote: > >David, > > > >to answer your question on IRC. > > > >yes the default patchlevel for gbuild is 3, because most fo the patch > >we had are like that... > really? > > Currently, in dmake build system all patches are applied with patch level 2. > The wrong default patch level (3) in gbuild UnpackedTarball.mk > gb_UnpackedTarball_PATCHLEVEL_DEFAULT := 3 > wasn't show up until rhino gbuildification, because in rhino there are two > build.xml files in different locations. With wrong patch level 3 the > wrong build.xml > was picked up and rejected. > > grep patch in solenv/inc give that: > tg_ext.mk:[...] patch $(PATCHFLAGS) -p2 [...] Yes, but notice from _where_ the patch is applied. In gbuild it is from the project's unpacked dir, while in dmake it is one level up. Therefore in gbuild we have to strip one level more. > > To fix that in gbuild would be as easy as: > gb_UnpackedTarball_PATCHLEVEL_DEFAULT := 2 Which would break apache-commons, beanshell and all other modules converted up to now that have old-style patches in them .-) D. ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [PATCH] gbuildification of rhino
Hi Norbert, On 03.10.2012 01:09, Norbert Thiebaud wrote: David, to answer your question on IRC. yes the default patchlevel for gbuild is 3, because most fo the patch we had are like that... really? Currently, in dmake build system all patches are applied with patch level 2. The wrong default patch level (3) in gbuild UnpackedTarball.mk gb_UnpackedTarball_PATCHLEVEL_DEFAULT := 3 wasn't show up until rhino gbuildification, because in rhino there are two build.xml files in different locations. With wrong patch level 3 the wrong build.xml was picked up and rejected. grep patch in solenv/inc give that: tg_ext.mk:[...] patch $(PATCHFLAGS) -p2 [...] To fix that in gbuild would be as easy as: gb_UnpackedTarball_PATCHLEVEL_DEFAULT := 2 With that fix in place, we could just remove $(eval $(call gb_UnpackedTarball_set_patchlevel,rhino,2)) from UnpackedTarball_rhino.mk Regards David ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [PATCH] gbuildification of rhino
David, to answer your question on IRC. yes the default patchlevel for gbuild is 3, because most fo the patch we had are like that... BUT, I pushed a patch some time ago so that if a patch is named *. where n is from 0-9, then gbuild will apply that patch with the patchlevel n (regardless of the default patchlevel or even of the level set by gb_UnpackedTarball_set_patchlevel() see: http://wiki.documentfoundation.org/Development/Patching_External_Libraries I strongly recommend that we use that naming convention rather than use the api call to override the default. so the cleaner solution is just the rename the patch Norbert ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
[PATCH] gbuildification of rhino
Hi, I have submitted a patch for review: https://gerrit.libreoffice.org/752 To pull it, you can do: git pull ssh://gerrit.libreoffice.org:29418/core refs/changes/52/752/1 gbuildification of rhino Default patch level 3 was not working here and must be overriden with gb_UnpackedTarball_set_patchlevel function with 2. New java file OfficeScriptInfo.java was extracted from rhino1_5R5.patch and added with gb_UnpackedTarball_add_file. Change-Id: Iba25b2350caffd943af93f59fa2139332b009ef2 --- M Makefile.top M Module_tail_build.mk M config_host.mk.in M configure.in A rhino/ExternalProject_rhino.mk A rhino/Makefile A rhino/Module_rhino.mk A rhino/OfficeScriptInfo.java A rhino/Package_rhino.mk A rhino/UnpackedTarball_rhino.mk D rhino/makefile.mk M rhino/prj/build.lst M rhino/prj/d.lst D rhino/prj/dmake M rhino/rhino1_5R5.patch M tail_build/prj/build.lst 16 files changed, 244 insertions(+), 216 deletions(-) -- To view, visit https://gerrit.libreoffice.org/752 To unsubscribe, visit https://gerrit.libreoffice.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Iba25b2350caffd943af93f59fa2139332b009ef2 Gerrit-PatchSet: 1 Gerrit-Project: core Gerrit-Branch: master Gerrit-Owner: David Ostrovsky ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
