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.000000000
+0100
+++ misc/build/rhino1_5R5/toolsrc/build.xml 2009-01-17
20:46:44.000000000 +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.