Re: Review Request: 8004151: build-infra: Generating X11 wrapper offset file is not cross compilable (AWT folks look here!)

[David Holmes]

Hi Erik,

I need this pushed to jdk8/build yesterday! :) Seriously!

This is now a blocker for the Profiles integration work. I thought I was 
utilizing 8005855 as a workaround but it never got pushed - instead I 
still have my hack of deleting the X_LIBS and X_CFLAGS from the sizer 
build. I can't have that hack in the repo that will go to PIT this week 
for the Profiles work, but I need something to get cross-compilation 
working.


Thanks,
David

On 19/01/2013 12:05 AM, Erik Joelsson wrote:

Here is a new webrev with my sorts incorporated. This has been baking
for quite a while in build-infra now and seems to be working.

http://cr.openjdk.java.net/~erikj/8004151/webrev.jdk.04/

/Erik

On 2012-12-07 11:05, Erik Joelsson wrote:

(resending to full recepients list)

I just hit another issue. I tried using a jdk8 boot-jdk and the order
of the output was different in the generated sizes file compared to
the one in the repo. Sorting both resulted in a clean diff.

/Erik

On 2012-12-05 14:20, Fredrik Öhrström wrote:

2012-11-29 15:54, Fredrik Öhrström skrev:

2012-11-29 15:36, Erik Joelsson skrev:

I just submitted a patch to build-infra for the dual generation on
all platforms since it breaks comparisons between old and new
build. In general, we can't change behavior in new build without
also changing the old before the old is removed.

Removing sizes.64-solaris-i386 breaks the old build on solaris-x86,
so I have readded it to build-infra.


Thanks Erik! The new and updated webrev is here:

http://cr.openjdk.java.net/~ohrstrom/webrev-8004151-gensrcX11wrapper-v2/




Ok, new webrev incorporating a few more Erik changes. Any comments
from the AWT experts?
http://cr.openjdk.java.net/~ohrstrom/webrev-8004151-gensrcX11wrapper-v3/

//Fredrik

Re: Review Request: 8004151: build-infra: Generating X11 wrapper offset file is not cross compilable (AWT folks look here!)

[Erik Joelsson]

Fixed both issues reported below

http://cr.openjdk.java.net/~erikj/8004151/webrev.jdk.05/

/Erik

On 2013-01-21 09:28, Fredrik Öhrström wrote:

double slash here: -I$(JDK_TOPDIR)//src/share/native/common

2013/1/21 David Holmes:

Minor typo in the gmk file

  32 # These are the once used.

once ->  ones

Thanks,
David


On 19/01/2013 12:05 AM, Erik Joelsson wrote:

Here is a new webrev with my sorts incorporated. This has been baking
for quite a while in build-infra now and seems to be working.

http://cr.openjdk.java.net/~erikj/8004151/webrev.jdk.04/

/Erik

On 2012-12-07 11:05, Erik Joelsson wrote:

(resending to full recepients list)

I just hit another issue. I tried using a jdk8 boot-jdk and the order
of the output was different in the generated sizes file compared to
the one in the repo. Sorting both resulted in a clean diff.

/Erik

On 2012-12-05 14:20, Fredrik Öhrström wrote:

2012-11-29 15:54, Fredrik Öhrström skrev:

2012-11-29 15:36, Erik Joelsson skrev:

I just submitted a patch to build-infra for the dual generation on
all platforms since it breaks comparisons between old and new
build. In general, we can't change behavior in new build without
also changing the old before the old is removed.

Removing sizes.64-solaris-i386 breaks the old build on solaris-x86,
so I have readded it to build-infra.


Thanks Erik! The new and updated webrev is here:


http://cr.openjdk.java.net/~ohrstrom/webrev-8004151-gensrcX11wrapper-v2/





Ok, new webrev incorporating a few more Erik changes. Any comments
from the AWT experts?
http://cr.openjdk.java.net/~ohrstrom/webrev-8004151-gensrcX11wrapper-v3/

//Fredrik

Re: Review Request: 8004151: build-infra: Generating X11 wrapper offset file is not cross compilable (AWT folks look here!)

[Fredrik Öhrström]

double slash here: -I$(JDK_TOPDIR)//src/share/native/common

2013/1/21 David Holmes :
> Minor typo in the gmk file
>
>  32 # These are the once used.
>
> once -> ones
>
> Thanks,
> David
>
>
> On 19/01/2013 12:05 AM, Erik Joelsson wrote:
>>
>> Here is a new webrev with my sorts incorporated. This has been baking
>> for quite a while in build-infra now and seems to be working.
>>
>> http://cr.openjdk.java.net/~erikj/8004151/webrev.jdk.04/
>>
>> /Erik
>>
>> On 2012-12-07 11:05, Erik Joelsson wrote:
>>>
>>> (resending to full recepients list)
>>>
>>> I just hit another issue. I tried using a jdk8 boot-jdk and the order
>>> of the output was different in the generated sizes file compared to
>>> the one in the repo. Sorting both resulted in a clean diff.
>>>
>>> /Erik
>>>
>>> On 2012-12-05 14:20, Fredrik Öhrström wrote:

 2012-11-29 15:54, Fredrik Öhrström skrev:
>
> 2012-11-29 15:36, Erik Joelsson skrev:
>>
>> I just submitted a patch to build-infra for the dual generation on
>> all platforms since it breaks comparisons between old and new
>> build. In general, we can't change behavior in new build without
>> also changing the old before the old is removed.
>>
>> Removing sizes.64-solaris-i386 breaks the old build on solaris-x86,
>> so I have readded it to build-infra.
>
>
> Thanks Erik! The new and updated webrev is here:
>
>
> http://cr.openjdk.java.net/~ohrstrom/webrev-8004151-gensrcX11wrapper-v2/
>
> 
>
>
 Ok, new webrev incorporating a few more Erik changes. Any comments
 from the AWT experts?
 http://cr.openjdk.java.net/~ohrstrom/webrev-8004151-gensrcX11wrapper-v3/

 //Fredrik

Re: Review Request: 8004151: build-infra: Generating X11 wrapper offset file is not cross compilable (AWT folks look here!)

[David Holmes]

Minor typo in the gmk file

 32 # These are the once used.

once -> ones

Thanks,
David

On 19/01/2013 12:05 AM, Erik Joelsson wrote:

Here is a new webrev with my sorts incorporated. This has been baking
for quite a while in build-infra now and seems to be working.

http://cr.openjdk.java.net/~erikj/8004151/webrev.jdk.04/

/Erik

On 2012-12-07 11:05, Erik Joelsson wrote:

(resending to full recepients list)

I just hit another issue. I tried using a jdk8 boot-jdk and the order
of the output was different in the generated sizes file compared to
the one in the repo. Sorting both resulted in a clean diff.

/Erik

On 2012-12-05 14:20, Fredrik Öhrström wrote:

2012-11-29 15:54, Fredrik Öhrström skrev:

2012-11-29 15:36, Erik Joelsson skrev:

I just submitted a patch to build-infra for the dual generation on
all platforms since it breaks comparisons between old and new
build. In general, we can't change behavior in new build without
also changing the old before the old is removed.

Removing sizes.64-solaris-i386 breaks the old build on solaris-x86,
so I have readded it to build-infra.


Thanks Erik! The new and updated webrev is here:

http://cr.openjdk.java.net/~ohrstrom/webrev-8004151-gensrcX11wrapper-v2/




Ok, new webrev incorporating a few more Erik changes. Any comments
from the AWT experts?
http://cr.openjdk.java.net/~ohrstrom/webrev-8004151-gensrcX11wrapper-v3/

//Fredrik

Re: Review Request: 8004151: build-infra: Generating X11 wrapper offset file is not cross compilable (AWT folks look here!)

[Erik Joelsson]

Here is a new webrev with my sorts incorporated. This has been baking 
for quite a while in build-infra now and seems to be working.


http://cr.openjdk.java.net/~erikj/8004151/webrev.jdk.04/

/Erik

On 2012-12-07 11:05, Erik Joelsson wrote:

(resending to full recepients list)

I just hit another issue. I tried using a jdk8 boot-jdk and the order 
of the output was different in the generated sizes file compared to 
the one in the repo. Sorting both resulted in a clean diff.


/Erik

On 2012-12-05 14:20, Fredrik Öhrström wrote:

2012-11-29 15:54, Fredrik Öhrström skrev:

2012-11-29 15:36, Erik Joelsson skrev:
I just submitted a patch to build-infra for the dual generation on 
all platforms since it breaks comparisons between old and new 
build. In general, we can't change behavior in new build without 
also changing the old before the old is removed.


Removing sizes.64-solaris-i386 breaks the old build on solaris-x86, 
so I have readded it to build-infra.


Thanks Erik! The new and updated webrev is here:

http://cr.openjdk.java.net/~ohrstrom/webrev-8004151-gensrcX11wrapper-v2/ 
 



Ok, new webrev incorporating a few more Erik changes. Any comments 
from the AWT experts?

http://cr.openjdk.java.net/~ohrstrom/webrev-8004151-gensrcX11wrapper-v3/

//Fredrik

Re: Review Request: 8004151: build-infra: Generating X11 wrapper offset file is not cross compilable (AWT folks look here!)

[Erik Joelsson]

(resending to full recepients list)

I just hit another issue. I tried using a jdk8 boot-jdk and the order of 
the output was different in the generated sizes file compared to the one 
in the repo. Sorting both resulted in a clean diff.


/Erik

On 2012-12-05 14:20, Fredrik Öhrström wrote:

2012-11-29 15:54, Fredrik Öhrström skrev:

2012-11-29 15:36, Erik Joelsson skrev:
I just submitted a patch to build-infra for the dual generation on 
all platforms since it breaks comparisons between old and new build. 
In general, we can't change behavior in new build without also 
changing the old before the old is removed.


Removing sizes.64-solaris-i386 breaks the old build on solaris-x86, 
so I have readded it to build-infra.


Thanks Erik! The new and updated webrev is here:

http://cr.openjdk.java.net/~ohrstrom/webrev-8004151-gensrcX11wrapper-v2/ 
 



Ok, new webrev incorporating a few more Erik changes. Any comments 
from the AWT experts?

http://cr.openjdk.java.net/~ohrstrom/webrev-8004151-gensrcX11wrapper-v3/

//Fredrik

Re: Review Request: 8004151: build-infra: Generating X11 wrapper offset file is not cross compilable (AWT folks look here!)

[Fredrik Öhrström]

2012-12-03 17:55, Kelly O'Hair skrev:

I notice that some of the $@.tmp stuff has been removed, along with the "$(RM) $@ 
$@.tmp" commands.
I know it's a pain to do this, but I have found it necessary, especially on 
Windows for some reason.


What reason? Can you give an example where it fails on windows?


The "$(MKDIR) -p $(@D)" guarantees that the destination directory exists, glad 
those remain.
The "$(RM) $@" is done to make sure the target file is removed so we get a 
fresh file. If some command
in the rules fail, I'd rather not see some partial $@ file left around from 
some previous build.
The use of $@.tmp files, followed by a "$(MV) $@.tmp $@" was to avoid any 
issues with a partially
constructed $@ file being left around, perhaps when some command in the rules 
got killed by a ^C or 'kill -9',
and a second make command thinking the file did not need to be built.


Make knows how to cleanup the goal file, if the recipe is interrupted 
halfway.


Take for example this makefile:

myprog.sh :
echo "#!/bin/sh" > $@
sleep 3
echo "echo Hello World!" >> $@
echo >> $@
chmod a+x $@

If you press ctrl-c during the sleep, then make will remove the half 
baked myprog.sh

and print: ^Cmake: *** Deleting file `myprog.sh'
make: *** [myprog.sh] Interrupt

However if you have an error in you recipe, for example add "cat 
not_there >> $@" at the end
of the recipe, then make will let the half baked  myprog.sh remain. But 
makefile execution

will of course terminate.

Thus if you have a recipe that will most likely not fail, then you 
should rely on make to do the
cleanup. If you have potential failures during a sequence of commands 
that construct the goal,
then the tmp trick should be used, to avoid incorrect goals remaining 
and confusing future incremental builds.


ALSO, if the recipe is triggered because the goal timestamp is older 
than the dependency timestamp,
then there is no need to remove the goal. If the goal is not 
regenerated, then a simple touch of the goal is enough.


On the windows file system, the timestamp resolution on FAT32 is 2 
seconds and can cause odd rounding
that (un)triggers goal dependency timestamp comparisons. This problems 
is solved by avoiding FAT32.


//Fredrik

Re: Review Request: 8004151: build-infra: Generating X11 wrapper offset file is not cross compilable (AWT folks look here!)

[Fredrik Öhrström]

2012-11-29 15:54, Fredrik Öhrström skrev:

2012-11-29 15:36, Erik Joelsson skrev:
I just submitted a patch to build-infra for the dual generation on 
all platforms since it breaks comparisons between old and new build. 
In general, we can't change behavior in new build without also 
changing the old before the old is removed.


Removing sizes.64-solaris-i386 breaks the old build on solaris-x86, 
so I have readded it to build-infra.


Thanks Erik! The new and updated webrev is here:

http://cr.openjdk.java.net/~ohrstrom/webrev-8004151-gensrcX11wrapper-v2/ 
 



Ok, new webrev incorporating a few more Erik changes. Any comments from 
the AWT experts?

http://cr.openjdk.java.net/~ohrstrom/webrev-8004151-gensrcX11wrapper-v3/

//Fredrik

Re: Review Request: 8004151: build-infra: Generating X11 wrapper offset file is not cross compilable (AWT folks look here!)

[Kelly O'Hair]

Something in the back of my mind says that there has to be a better solution to 
this X11 sizeof issue, but
at the same time, I don't think we need to find that solution now.

I notice that some of the $@.tmp stuff has been removed, along with the "$(RM) 
$@ $@.tmp" commands.
I know it's a pain to do this, but I have found it necessary, especially on 
Windows for some reason.

The "$(MKDIR) -p $(@D)" guarantees that the destination directory exists, glad 
those remain.
The "$(RM) $@" is done to make sure the target file is removed so we get a 
fresh file. If some command
in the rules fail, I'd rather not see some partial $@ file left around from 
some previous build.
The use of $@.tmp files, followed by a "$(MV) $@.tmp $@" was to avoid any 
issues with a partially
constructed $@ file being left around, perhaps when some command in the rules 
got killed by a ^C or 'kill -9',
and a second make command thinking the file did not need to be built.
I suppose these habits are paranoid, but it's what I've done in the past to 
guarantee predictability.
If there is a better way to do this I am all ears.

-kto

On Nov 29, 2012, at 6:27 AM, Fredrik Öhrström wrote:

> The GensrcX11Wrapper.gmk makefile creates a new sizes.32 or sizes.64 by 
> running a generated C program
> that performs many sizeof calculations on X11 structures. This is cross 
> compileable.
> 
> Fortunately the sizes.32 and sizes.64 offset files are very stable. Thus we 
> can commit them to the
> source code repository. When not cross compiling we perform a verification 
> step that
> regenerates the offset file for the build platforms bits, and checks that it 
> is the same
> as the committed version.
> 
> Patch is here:
> http://cr.openjdk.java.net/~ohrstrom/webrev-8004151-gensrcX11wrapper/
> (do not read the diff of the makefile, read the new makefile, too many 
> changes.)
> 
> Question: by always supplying both sizes.32 and sizes.64 to the generator 
> tool, it
> seems like it is encoding both offsets into the java classes. This might not 
> always
> have been done before. Good or bad?
> 
> //Fredrik

Re: Review Request: 8004151: build-infra: Generating X11 wrapper offset file is not cross compilable (AWT folks look here!)

[Fredrik Öhrström]

2012-11-29 15:36, Erik Joelsson skrev:
I just submitted a patch to build-infra for the dual generation on all 
platforms since it breaks comparisons between old and new build. In 
general, we can't change behavior in new build without also changing 
the old before the old is removed.


Removing sizes.64-solaris-i386 breaks the old build on solaris-x86, so 
I have readded it to build-infra.


Thanks Erik! The new and updated webrev is here:

http://cr.openjdk.java.net/~ohrstrom/webrev-8004151-gensrcX11wrapper-v2/ 

Re: Review Request: 8004151: build-infra: Generating X11 wrapper offset file is not cross compilable (AWT folks look here!)

[Fredrik Öhrström]

2012-11-29 15:27, Fredrik Öhrström skrev:
The GensrcX11Wrapper.gmk makefile creates a new sizes.32 or sizes.64 
by running a generated C program
that performs many sizeof calculations on X11 structures. This is 
cross compileable.
That "not" was accidently dropped. Obviously this is NOT cross 
compileable.

Re: Review Request: 8004151: build-infra: Generating X11 wrapper offset file is not cross compilable (AWT folks look here!)

[Erik Joelsson]

I just submitted a patch to build-infra for the dual generation on all 
platforms since it breaks comparisons between old and new build. In 
general, we can't change behavior in new build without also changing the 
old before the old is removed.


Removing sizes.64-solaris-i386 breaks the old build on solaris-x86, so I 
have readded it to build-infra.


/Erik

On 2012-11-29 15:27, Fredrik Öhrström wrote:
The GensrcX11Wrapper.gmk makefile creates a new sizes.32 or sizes.64 
by running a generated C program
that performs many sizeof calculations on X11 structures. This is 
cross compileable.


Fortunately the sizes.32 and sizes.64 offset files are very stable. 
Thus we can commit them to the
source code repository. When not cross compiling we perform a 
verification step that
regenerates the offset file for the build platforms bits, and checks 
that it is the same

as the committed version.

Patch is here:
http://cr.openjdk.java.net/~ohrstrom/webrev-8004151-gensrcX11wrapper/
(do not read the diff of the makefile, read the new makefile, too many 
changes.)


Question: by always supplying both sizes.32 and sizes.64 to the 
generator tool, it
seems like it is encoding both offsets into the java classes. This 
might not always

have been done before. Good or bad?

//Fredrik

Re: Review Request: 8004151: build-infra: Generating X11 wrapper offset file is not cross compilable (AWT folks look here!)

[Anthony Petrov]

CC'ing awt-dev@, Yuri, and Artem.

--
best regards,
Anthony

On 11/29/12 18:27, Fredrik Öhrström wrote:

The GensrcX11Wrapper.gmk makefile creates a new sizes.32 or sizes.64 by
running a generated C program
that performs many sizeof calculations on X11 structures. This is cross
compileable.

Fortunately the sizes.32 and sizes.64 offset files are very stable. Thus
we can commit them to the
source code repository. When not cross compiling we perform a
verification step that
regenerates the offset file for the build platforms bits, and checks
that it is the same
as the committed version.

Patch is here:
http://cr.openjdk.java.net/~ohrstrom/webrev-8004151-gensrcX11wrapper/
(do not read the diff of the makefile, read the new makefile, too many
changes.)

Question: by always supplying both sizes.32 and sizes.64 to the
generator tool, it
seems like it is encoding both offsets into the java classes. This might
not always
have been done before. Good or bad?

//Fredrik

Review Request: 8004151: build-infra: Generating X11 wrapper offset file is not cross compilable (AWT folks look here!)

[Fredrik Öhrström]

The GensrcX11Wrapper.gmk makefile creates a new sizes.32 or sizes.64 by 
running a generated C program
that performs many sizeof calculations on X11 structures. This is cross 
compileable.


Fortunately the sizes.32 and sizes.64 offset files are very stable. Thus 
we can commit them to the
source code repository. When not cross compiling we perform a 
verification step that
regenerates the offset file for the build platforms bits, and checks 
that it is the same

as the committed version.

Patch is here:
http://cr.openjdk.java.net/~ohrstrom/webrev-8004151-gensrcX11wrapper/
(do not read the diff of the makefile, read the new makefile, too many 
changes.)


Question: by always supplying both sizes.32 and sizes.64 to the 
generator tool, it
seems like it is encoding both offsets into the java classes. This might 
not always

have been done before. Good or bad?

//Fredrik