RFR(S): 8132704: [TESTBUG] jdk/internal/jimage/ExecutableTest.java incorrectly asserts all files to be executable
Hi, can somebody please review this test fix: http://cr.openjdk.java.net/~simonis/webrevs/2015/8132704/ https://bugs.openjdk.java.net/browse/JDK-8132704 The initial test checked that all the files in the bin/ directory are executable by everybody. Unfortunately this was too optimistic because in the closed build the bin/ directory contains configuration files which are not executable. The new version of the test uses a predefined static list of executables which are checked for the executable permissions if the corresponding files exist. Thank you and best regards, Volker
Hi Volker, Possibly the real bug is that there is non-executable file in the bin directory. There is a /conf directory which would probably be a better place for that. Roger On 7/30/2015 11:28 AM, Volker Simonis wrote:
Hi,
can somebody please review this test fix:
http://cr.openjdk.java.net/~simonis/webrevs/2015/8132704/ https://bugs.openjdk.java.net/browse/JDK-8132704
The initial test checked that all the files in the bin/ directory are executable by everybody. Unfortunately this was too optimistic because in the closed build the bin/ directory contains configuration files which are not executable.
The new version of the test uses a predefined static list of executables which are checked for the executable permissions if the corresponding files exist.
Thank you and best regards, Volker
On Thu, Jul 30, 2015 at 5:34 PM, Roger Riggs <Roger.Riggs@oracle.com> wrote:
Hi Volker,
Possibly the real bug is that there is non-executable file in the bin directory. There is a /conf directory which would probably be a better place for that.
Yes, I agree. But I thought you want a fast fix for the test failure :) Moving that config file is probably a bigger effort. Moreover the bin/ directory on Windows also contains .dll and .diz files. However on Windows, all the files seem to be executable (at least the test did succeed before). Nevertheless, checking only a known subset of executables seems safer and "good enough". What do you think? Volker
Roger
On 7/30/2015 11:28 AM, Volker Simonis wrote:
Hi,
can somebody please review this test fix:
http://cr.openjdk.java.net/~simonis/webrevs/2015/8132704/ https://bugs.openjdk.java.net/browse/JDK-8132704
The initial test checked that all the files in the bin/ directory are executable by everybody. Unfortunately this was too optimistic because in the closed build the bin/ directory contains configuration files which are not executable.
The new version of the test uses a predefined static list of executables which are checked for the executable permissions if the corresponding files exist.
Thank you and best regards, Volker
Hi Volker, There is already as task for JMC to move that file but who knows exactly when. A list of exception(s) would be better I think, because generally, everything in bin should be executable. WDYT? Roger On 7/30/2015 11:40 AM, Volker Simonis wrote:
On Thu, Jul 30, 2015 at 5:34 PM, Roger Riggs <Roger.Riggs@oracle.com> wrote:
Hi Volker,
Possibly the real bug is that there is non-executable file in the bin directory. There is a /conf directory which would probably be a better place for that.
Yes, I agree. But I thought you want a fast fix for the test failure :) Moving that config file is probably a bigger effort.
Moreover the bin/ directory on Windows also contains .dll and .diz files. However on Windows, all the files seem to be executable (at least the test did succeed before). Nevertheless, checking only a known subset of executables seems safer and "good enough".
What do you think? Volker
Roger
On 7/30/2015 11:28 AM, Volker Simonis wrote:
Hi,
can somebody please review this test fix:
http://cr.openjdk.java.net/~simonis/webrevs/2015/8132704/ https://bugs.openjdk.java.net/browse/JDK-8132704
The initial test checked that all the files in the bin/ directory are executable by everybody. Unfortunately this was too optimistic because in the closed build the bin/ directory contains configuration files which are not executable.
The new version of the test uses a predefined static list of executables which are checked for the executable permissions if the corresponding files exist.
Thank you and best regards, Volker
On Thu, Jul 30, 2015 at 5:47 PM, Roger Riggs <Roger.Riggs@oracle.com> wrote:
Hi Volker,
There is already as task for JMC to move that file but who knows exactly when.
A list of exception(s) would be better I think, because generally, everything in bin should be executable. WDYT?
OK, I'm fine with that. Unfortunately I'll have to hurry home now. I'll do the change tomorrow (or you can take over if it's urgent). Regards, Volker
Roger
On 7/30/2015 11:40 AM, Volker Simonis wrote:
On Thu, Jul 30, 2015 at 5:34 PM, Roger Riggs <Roger.Riggs@oracle.com> wrote:
Hi Volker,
Possibly the real bug is that there is non-executable file in the bin directory. There is a /conf directory which would probably be a better place for that.
Yes, I agree. But I thought you want a fast fix for the test failure :) Moving that config file is probably a bigger effort.
Moreover the bin/ directory on Windows also contains .dll and .diz files. However on Windows, all the files seem to be executable (at least the test did succeed before). Nevertheless, checking only a known subset of executables seems safer and "good enough".
What do you think? Volker
Roger
On 7/30/2015 11:28 AM, Volker Simonis wrote:
Hi,
can somebody please review this test fix:
http://cr.openjdk.java.net/~simonis/webrevs/2015/8132704/ https://bugs.openjdk.java.net/browse/JDK-8132704
The initial test checked that all the files in the bin/ directory are executable by everybody. Unfortunately this was too optimistic because in the closed build the bin/ directory contains configuration files which are not executable.
The new version of the test uses a predefined static list of executables which are checked for the executable permissions if the corresponding files exist.
Thank you and best regards, Volker
On Thu, Jul 30, 2015 at 5:49 PM, Volker Simonis <volker.simonis@gmail.com> wrote:
On Thu, Jul 30, 2015 at 5:47 PM, Roger Riggs <Roger.Riggs@oracle.com> wrote:
Hi Volker,
There is already as task for JMC to move that file but who knows exactly when.
A list of exception(s) would be better I think, because generally, everything in bin should be executable. WDYT?
OK, I'm fine with that. Unfortunately I'll have to hurry home now. I'll do the change tomorrow (or you can take over if it's urgent).
So here comes the new version: http://cr.openjdk.java.net/~simonis/webrevs/2015/8132704.v2 I've added 'jmc.ini' to the exclude list for now. Could you please check if there are other files I should add to the list before pushing? Do you have a bug ID for the task to move 'jmc.ini' out of the bin direcotry. I think it should be updated with a link to this bug and with the info to remove 'jmc.ini' from the exclude list once the file has been moved away from bin. Regards, Volker
Regards, Volker
Roger
On 7/30/2015 11:40 AM, Volker Simonis wrote:
On Thu, Jul 30, 2015 at 5:34 PM, Roger Riggs <Roger.Riggs@oracle.com> wrote:
Hi Volker,
Possibly the real bug is that there is non-executable file in the bin directory. There is a /conf directory which would probably be a better place for that.
Yes, I agree. But I thought you want a fast fix for the test failure :) Moving that config file is probably a bigger effort.
Moreover the bin/ directory on Windows also contains .dll and .diz files. However on Windows, all the files seem to be executable (at least the test did succeed before). Nevertheless, checking only a known subset of executables seems safer and "good enough".
What do you think? Volker
Roger
On 7/30/2015 11:28 AM, Volker Simonis wrote:
Hi,
can somebody please review this test fix:
http://cr.openjdk.java.net/~simonis/webrevs/2015/8132704/ https://bugs.openjdk.java.net/browse/JDK-8132704
The initial test checked that all the files in the bin/ directory are executable by everybody. Unfortunately this was too optimistic because in the closed build the bin/ directory contains configuration files which are not executable.
The new version of the test uses a predefined static list of executables which are checked for the executable permissions if the corresponding files exist.
Thank you and best regards, Volker
On 31/07/2015 11:12, Volker Simonis wrote:
:
So here comes the new version:
http://cr.openjdk.java.net/~simonis/webrevs/2015/8132704.v2
I've added 'jmc.ini' to the exclude list for now. Could you please check if there are other files I should add to the list before pushing?
Do you have a bug ID for the task to move 'jmc.ini' out of the bin direcotry. I think it should be updated with a link to this bug and with the info to remove 'jmc.ini' from the exclude list once the file has been moved away from bin.
There is a linked issue but it's to a proprietary product that you won't see in JIRA so I think you won't see the link. Products that get overlay over the modular image need to respect the layout but I don't know the ETA on when this will be moved to the conf directory. Your patch looks okay but a bit embarrassing that you've had to do this. -Alan.
On Fri, Jul 31, 2015 at 12:30 PM, Alan Bateman <Alan.Bateman@oracle.com> wrote:
On 31/07/2015 11:12, Volker Simonis wrote:
:
So here comes the new version:
http://cr.openjdk.java.net/~simonis/webrevs/2015/8132704.v2
I've added 'jmc.ini' to the exclude list for now. Could you please check if there are other files I should add to the list before pushing?
Do you have a bug ID for the task to move 'jmc.ini' out of the bin direcotry. I think it should be updated with a link to this bug and with the info to remove 'jmc.ini' from the exclude list once the file has been moved away from bin.
There is a linked issue but it's to a proprietary product that you won't see in JIRA so I think you won't see the link. Products that get overlay over the modular image need to respect the layout but I don't know the ETA on when this will be moved to the conf directory.
Your patch looks okay but a bit embarrassing that you've had to do this.
That's no problem - you're welcome. Sometimes you're helping out with AIX as well :) Regards, Volker -Alan.
On 31 Jul 2015, at 11:12, Volker Simonis <volker.simonis@gmail.com> wrote:
On Thu, Jul 30, 2015 at 5:49 PM, Volker Simonis <volker.simonis@gmail.com <mailto:volker.simonis@gmail.com>> wrote:
On Thu, Jul 30, 2015 at 5:47 PM, Roger Riggs <Roger.Riggs@oracle.com> wrote:
Hi Volker,
There is already as task for JMC to move that file but who knows exactly when.
A list of exception(s) would be better I think, because generally, everything in bin should be executable. WDYT?
OK, I'm fine with that. Unfortunately I'll have to hurry home now. I'll do the change tomorrow (or you can take over if it's urgent).
So here comes the new version:
http://cr.openjdk.java.net/~simonis/webrevs/2015/8132704.v2 <http://cr.openjdk.java.net/~simonis/webrevs/2015/8132704.v2>
This change looks good to me Volker.
I've added 'jmc.ini' to the exclude list for now. Could you please check if there are other files I should add to the list before pushing?
I believe that jmc.ini is the only one.
Do you have a bug ID for the task to move 'jmc.ini' out of the bin direcotry. I think it should be updated with a link to this bug and with the info to remove 'jmc.ini' from the exclude list once the file has been moved away from bin.
-Chris.
Regards, Volker
Regards, Volker
Roger
On 7/30/2015 11:40 AM, Volker Simonis wrote:
On Thu, Jul 30, 2015 at 5:34 PM, Roger Riggs <Roger.Riggs@oracle.com> wrote:
Hi Volker,
Possibly the real bug is that there is non-executable file in the bin directory. There is a /conf directory which would probably be a better place for that.
Yes, I agree. But I thought you want a fast fix for the test failure :) Moving that config file is probably a bigger effort.
Moreover the bin/ directory on Windows also contains .dll and .diz files. However on Windows, all the files seem to be executable (at least the test did succeed before). Nevertheless, checking only a known subset of executables seems safer and "good enough".
What do you think? Volker
Roger
On 7/30/2015 11:28 AM, Volker Simonis wrote:
Hi,
can somebody please review this test fix:
http://cr.openjdk.java.net/~simonis/webrevs/2015/8132704/ https://bugs.openjdk.java.net/browse/JDK-8132704
The initial test checked that all the files in the bin/ directory are executable by everybody. Unfortunately this was too optimistic because in the closed build the bin/ directory contains configuration files which are not executable.
The new version of the test uses a predefined static list of executables which are checked for the executable permissions if the corresponding files exist.
Thank you and best regards, Volker
On Fri, Jul 31, 2015 at 12:32 PM, Chris Hegarty <chris.hegarty@oracle.com> wrote:
On 31 Jul 2015, at 11:12, Volker Simonis <volker.simonis@gmail.com> wrote:
On Thu, Jul 30, 2015 at 5:49 PM, Volker Simonis <volker.simonis@gmail.com> wrote:
On Thu, Jul 30, 2015 at 5:47 PM, Roger Riggs <Roger.Riggs@oracle.com> wrote:
Hi Volker,
There is already as task for JMC to move that file but who knows exactly when.
A list of exception(s) would be better I think, because generally, everything in bin should be executable. WDYT?
OK, I'm fine with that. Unfortunately I'll have to hurry home now. I'll do the change tomorrow (or you can take over if it's urgent).
So here comes the new version:
http://cr.openjdk.java.net/~simonis/webrevs/2015/8132704.v2
This change looks good to me Volker.
Thanks. I'll push it now.
Regards, Volker
I've added 'jmc.ini' to the exclude list for now. Could you please check if there are other files I should add to the list before pushing?
I believe that jmc.ini is the only one.
Do you have a bug ID for the task to move 'jmc.ini' out of the bin direcotry. I think it should be updated with a link to this bug and with the info to remove 'jmc.ini' from the exclude list once the file has been moved away from bin.
-Chris.
Regards, Volker
Regards, Volker
Roger
On 7/30/2015 11:40 AM, Volker Simonis wrote:
On Thu, Jul 30, 2015 at 5:34 PM, Roger Riggs <Roger.Riggs@oracle.com> wrote:
Hi Volker,
Possibly the real bug is that there is non-executable file in the bin directory. There is a /conf directory which would probably be a better place for that.
Yes, I agree. But I thought you want a fast fix for the test failure :) Moving that config file is probably a bigger effort.
Moreover the bin/ directory on Windows also contains .dll and .diz files. However on Windows, all the files seem to be executable (at least the test did succeed before). Nevertheless, checking only a known subset of executables seems safer and "good enough".
What do you think? Volker
Roger
On 7/30/2015 11:28 AM, Volker Simonis wrote:
Hi,
can somebody please review this test fix:
http://cr.openjdk.java.net/~simonis/webrevs/2015/8132704/ https://bugs.openjdk.java.net/browse/JDK-8132704
The initial test checked that all the files in the bin/ directory are executable by everybody. Unfortunately this was too optimistic because in the closed build the bin/ directory contains configuration files which are not executable.
The new version of the test uses a predefined static list of executables which are checked for the executable permissions if the corresponding files exist.
Thank you and best regards, Volker
participants (4)
-
Alan Bateman
-
Chris Hegarty
-
Roger Riggs
-
Volker Simonis