Re: RFR 6997010: Consolidate java.security files into one file with modifications
On 2014-08-05 23:24, Sean Mullan wrote: On 07/28/2014 09:53 AM, Wang Weijun wrote: Yes, you are right. Webrev updated at http://cr.openjdk.java.net/~weijun/6997010/webrev.02. GendataJavaSecurity.gmk and MakeJavaSecurity.java updated. There's an unnecessary indent at line 1 of GendataJavaSecurity.gmk Speaking of indentation, please also change the ifdef bodies to 2 spaces. /Erik In CheckSecurityProvider.java, ucrypto is in the closed sources, so Security.getProviders() will not return it if you are testing an OpenJDK build. You need to adjust the test to not include this provider if testing an OpenJDK build. --Sean Thanks Max On Jul 28, 2014, at 19:43, Erik Joelsson erik.joels...@oracle.com wrote: Hello Max, Shouldn't the rule for $(GENDATA_JAVA_SECURITY) depend on $(RESTRICTED_PKGS_SRC) so that updates to the pkgs file triggers a rebuild? For that to work, the variable $(RESTRICTED_PKGS_SRC) needs to be empty for the OPENJDK case rather than have a dummy name and MakeJavaSecurity.java needs to handle missing the last argument. /Erik On 2014-07-28 03:44, Wang Weijun wrote: Webrev updated at http://cr.openjdk.java.net/~weijun/6997010/webrev.01/ New test CheckSecurityProvider.java, and updates to MakeJavaSecurity.addPackages(). Thanks Max On Jul 25, 2014, at 22:44, Wang Weijun weijun.w...@oracle.com wrote: On Jul 25, 2014, at 22:30, Sean Mullan sean.mul...@oracle.com wrote: http://cr.openjdk.java.net/~weijun/6997010/webrev.00/ 4. *IMPORTANT*: In order to easily maintain platform-related entries, every line (including the last line) in package.access and package.definition MUST end with ',\' now. A blank line MUST exist after the last line. This avoid ugly lines like #ifndef windows entry1. #endif #ifdef windows entry1.,\ entry2 #endif What happens if someone (inevitably) adds a new package to the list and forgets to do either of these? Does it result in a build failure? No build failure, but test/java/security/SecurityManager/CheckPackageAccess.java will fail. I can add check in the build tool. Otherwise looks good, although I think it would be useful to write an additional test to make sure the correct providers are installed and ordered correctly on the different platforms, something similar to the java/lang/SecurityManager/CheckPackageAccess.java test but specific to providers. OK. Thanks Max
Re: RFR 6997010: Consolidate java.security files into one file with modifications
On 8/6/2014 17:04, Erik Joelsson wrote: Speaking of indentation, please also change the ifdef bodies to 2 spaces. Sure. Whenever I edit a Makefile, I dare not use spaces and always use TABs. Obviously an ifdef does not need it. Thanks Max
Re: RFR 6997010: Consolidate java.security files into one file with modifications
On 2014-08-06 11:14, Weijun Wang wrote: On 8/6/2014 17:04, Erik Joelsson wrote: Speaking of indentation, please also change the ifdef bodies to 2 spaces. Sure. Whenever I edit a Makefile, I dare not use spaces and always use TABs. Obviously an ifdef does not need it. The tab character unfortunately has significance in make, so we carefully differentiate between using space and tab when it's appropriate. A tab in the wrong place can also cause trouble. /Erik Thanks Max
Re: RFR 6997010: Consolidate java.security files into one file with modifications
On 07/28/2014 09:53 AM, Wang Weijun wrote: Yes, you are right. Webrev updated at http://cr.openjdk.java.net/~weijun/6997010/webrev.02. GendataJavaSecurity.gmk and MakeJavaSecurity.java updated. There's an unnecessary indent at line 1 of GendataJavaSecurity.gmk In CheckSecurityProvider.java, ucrypto is in the closed sources, so Security.getProviders() will not return it if you are testing an OpenJDK build. You need to adjust the test to not include this provider if testing an OpenJDK build. --Sean Thanks Max On Jul 28, 2014, at 19:43, Erik Joelsson erik.joels...@oracle.com wrote: Hello Max, Shouldn't the rule for $(GENDATA_JAVA_SECURITY) depend on $(RESTRICTED_PKGS_SRC) so that updates to the pkgs file triggers a rebuild? For that to work, the variable $(RESTRICTED_PKGS_SRC) needs to be empty for the OPENJDK case rather than have a dummy name and MakeJavaSecurity.java needs to handle missing the last argument. /Erik On 2014-07-28 03:44, Wang Weijun wrote: Webrev updated at http://cr.openjdk.java.net/~weijun/6997010/webrev.01/ New test CheckSecurityProvider.java, and updates to MakeJavaSecurity.addPackages(). Thanks Max On Jul 25, 2014, at 22:44, Wang Weijun weijun.w...@oracle.com wrote: On Jul 25, 2014, at 22:30, Sean Mullan sean.mul...@oracle.com wrote: http://cr.openjdk.java.net/~weijun/6997010/webrev.00/ 4. *IMPORTANT*: In order to easily maintain platform-related entries, every line (including the last line) in package.access and package.definition MUST end with ',\' now. A blank line MUST exist after the last line. This avoid ugly lines like #ifndef windows entry1. #endif #ifdef windows entry1.,\ entry2 #endif What happens if someone (inevitably) adds a new package to the list and forgets to do either of these? Does it result in a build failure? No build failure, but test/java/security/SecurityManager/CheckPackageAccess.java will fail. I can add check in the build tool. Otherwise looks good, although I think it would be useful to write an additional test to make sure the correct providers are installed and ordered correctly on the different platforms, something similar to the java/lang/SecurityManager/CheckPackageAccess.java test but specific to providers. OK. Thanks Max
Re: RFR 6997010: Consolidate java.security files into one file with modifications
Hello Max, Shouldn't the rule for $(GENDATA_JAVA_SECURITY) depend on $(RESTRICTED_PKGS_SRC) so that updates to the pkgs file triggers a rebuild? For that to work, the variable $(RESTRICTED_PKGS_SRC) needs to be empty for the OPENJDK case rather than have a dummy name and MakeJavaSecurity.java needs to handle missing the last argument. /Erik On 2014-07-28 03:44, Wang Weijun wrote: Webrev updated at http://cr.openjdk.java.net/~weijun/6997010/webrev.01/ New test CheckSecurityProvider.java, and updates to MakeJavaSecurity.addPackages(). Thanks Max On Jul 25, 2014, at 22:44, Wang Weijun weijun.w...@oracle.com wrote: On Jul 25, 2014, at 22:30, Sean Mullan sean.mul...@oracle.com wrote: http://cr.openjdk.java.net/~weijun/6997010/webrev.00/ 4. *IMPORTANT*: In order to easily maintain platform-related entries, every line (including the last line) in package.access and package.definition MUST end with ',\' now. A blank line MUST exist after the last line. This avoid ugly lines like #ifndef windows entry1. #endif #ifdef windows entry1.,\ entry2 #endif What happens if someone (inevitably) adds a new package to the list and forgets to do either of these? Does it result in a build failure? No build failure, but test/java/security/SecurityManager/CheckPackageAccess.java will fail. I can add check in the build tool. Otherwise looks good, although I think it would be useful to write an additional test to make sure the correct providers are installed and ordered correctly on the different platforms, something similar to the java/lang/SecurityManager/CheckPackageAccess.java test but specific to providers. OK. Thanks Max
Re: RFR 6997010: Consolidate java.security files into one file with modifications
Yes, you are right. Webrev updated at http://cr.openjdk.java.net/~weijun/6997010/webrev.02. GendataJavaSecurity.gmk and MakeJavaSecurity.java updated. Thanks Max On Jul 28, 2014, at 19:43, Erik Joelsson erik.joels...@oracle.com wrote: Hello Max, Shouldn't the rule for $(GENDATA_JAVA_SECURITY) depend on $(RESTRICTED_PKGS_SRC) so that updates to the pkgs file triggers a rebuild? For that to work, the variable $(RESTRICTED_PKGS_SRC) needs to be empty for the OPENJDK case rather than have a dummy name and MakeJavaSecurity.java needs to handle missing the last argument. /Erik On 2014-07-28 03:44, Wang Weijun wrote: Webrev updated at http://cr.openjdk.java.net/~weijun/6997010/webrev.01/ New test CheckSecurityProvider.java, and updates to MakeJavaSecurity.addPackages(). Thanks Max On Jul 25, 2014, at 22:44, Wang Weijun weijun.w...@oracle.com wrote: On Jul 25, 2014, at 22:30, Sean Mullan sean.mul...@oracle.com wrote: http://cr.openjdk.java.net/~weijun/6997010/webrev.00/ 4. *IMPORTANT*: In order to easily maintain platform-related entries, every line (including the last line) in package.access and package.definition MUST end with ',\' now. A blank line MUST exist after the last line. This avoid ugly lines like #ifndef windows entry1. #endif #ifdef windows entry1.,\ entry2 #endif What happens if someone (inevitably) adds a new package to the list and forgets to do either of these? Does it result in a build failure? No build failure, but test/java/security/SecurityManager/CheckPackageAccess.java will fail. I can add check in the build tool. Otherwise looks good, although I think it would be useful to write an additional test to make sure the correct providers are installed and ordered correctly on the different platforms, something similar to the java/lang/SecurityManager/CheckPackageAccess.java test but specific to providers. OK. Thanks Max
Re: RFR 6997010: Consolidate java.security files into one file with modifications
Build change looks good to me now. /Erik On 2014-07-28 15:53, Wang Weijun wrote: Yes, you are right. Webrev updated at http://cr.openjdk.java.net/~weijun/6997010/webrev.02. GendataJavaSecurity.gmk and MakeJavaSecurity.java updated. Thanks Max On Jul 28, 2014, at 19:43, Erik Joelsson erik.joels...@oracle.com wrote: Hello Max, Shouldn't the rule for $(GENDATA_JAVA_SECURITY) depend on $(RESTRICTED_PKGS_SRC) so that updates to the pkgs file triggers a rebuild? For that to work, the variable $(RESTRICTED_PKGS_SRC) needs to be empty for the OPENJDK case rather than have a dummy name and MakeJavaSecurity.java needs to handle missing the last argument. /Erik On 2014-07-28 03:44, Wang Weijun wrote: Webrev updated at http://cr.openjdk.java.net/~weijun/6997010/webrev.01/ New test CheckSecurityProvider.java, and updates to MakeJavaSecurity.addPackages(). Thanks Max On Jul 25, 2014, at 22:44, Wang Weijun weijun.w...@oracle.com wrote: On Jul 25, 2014, at 22:30, Sean Mullan sean.mul...@oracle.com wrote: http://cr.openjdk.java.net/~weijun/6997010/webrev.00/ 4. *IMPORTANT*: In order to easily maintain platform-related entries, every line (including the last line) in package.access and package.definition MUST end with ',\' now. A blank line MUST exist after the last line. This avoid ugly lines like #ifndef windows entry1. #endif #ifdef windows entry1.,\ entry2 #endif What happens if someone (inevitably) adds a new package to the list and forgets to do either of these? Does it result in a build failure? No build failure, but test/java/security/SecurityManager/CheckPackageAccess.java will fail. I can add check in the build tool. Otherwise looks good, although I think it would be useful to write an additional test to make sure the correct providers are installed and ordered correctly on the different platforms, something similar to the java/lang/SecurityManager/CheckPackageAccess.java test but specific to providers. OK. Thanks Max
Re: RFR 6997010: Consolidate java.security files into one file with modifications
Webrev updated at http://cr.openjdk.java.net/~weijun/6997010/webrev.01/ New test CheckSecurityProvider.java, and updates to MakeJavaSecurity.addPackages(). Thanks Max On Jul 25, 2014, at 22:44, Wang Weijun weijun.w...@oracle.com wrote: On Jul 25, 2014, at 22:30, Sean Mullan sean.mul...@oracle.com wrote: http://cr.openjdk.java.net/~weijun/6997010/webrev.00/ 4. *IMPORTANT*: In order to easily maintain platform-related entries, every line (including the last line) in package.access and package.definition MUST end with ',\' now. A blank line MUST exist after the last line. This avoid ugly lines like #ifndef windows entry1. #endif #ifdef windows entry1.,\ entry2 #endif What happens if someone (inevitably) adds a new package to the list and forgets to do either of these? Does it result in a build failure? No build failure, but test/java/security/SecurityManager/CheckPackageAccess.java will fail. I can add check in the build tool. Otherwise looks good, although I think it would be useful to write an additional test to make sure the correct providers are installed and ordered correctly on the different platforms, something similar to the java/lang/SecurityManager/CheckPackageAccess.java test but specific to providers. OK. Thanks Max
Re: RFR 6997010: Consolidate java.security files into one file with modifications
On 07/22/2014 10:28 PM, Wang Weijun wrote: Please review the code change at http://cr.openjdk.java.net/~weijun/6997010/webrev.00/ The fix consolidates java.security-platform files into one with #ifdef directives. There are several major changes: 1. Creation of file is moved from CopyFiles to GenerateData, since we are really generating something now. Said that, the source data is kept in src/share/lib/security instead of make/data. I am OK with moving it if anyone desires. 2. The new tool MakeJavaSecurity includes the function of old AddToRestrictedPkgs. MakeJavaSecurity includes a new argument to deal with the platform dependent entries. The restricted.pkgs argument is also changed from a list of entries to a file name, so that we can also support the same #ifdef mechanism inside restricted.pkgs. 3. The new consolidated java.security supports #ifdef and #ifndef. It is not necessary to support #else or (and|or) of multiple #ifdef's now. 4. *IMPORTANT*: In order to easily maintain platform-related entries, every line (including the last line) in package.access and package.definition MUST end with ',\' now. A blank line MUST exist after the last line. This avoid ugly lines like #ifndef windows entry1. #endif #ifdef windows entry1.,\ entry2 #endif What happens if someone (inevitably) adds a new package to the list and forgets to do either of these? Does it result in a build failure? Otherwise looks good, although I think it would be useful to write an additional test to make sure the correct providers are installed and ordered correctly on the different platforms, something similar to the java/lang/SecurityManager/CheckPackageAccess.java test but specific to providers. Thanks for doing this, it will really help maintaining this file going forward! --Sean The MakeJavaSecurity tool will strip the trailing ,\ from the last line to make the file exactly the same as before, although personally I don't think it's really necessary since the following empty line will terminate the entry automatically. Thanks Max
Re: RFR 6997010: Consolidate java.security files into one file with modifications
On Jul 25, 2014, at 22:30, Sean Mullan sean.mul...@oracle.com wrote: http://cr.openjdk.java.net/~weijun/6997010/webrev.00/ 4. *IMPORTANT*: In order to easily maintain platform-related entries, every line (including the last line) in package.access and package.definition MUST end with ',\' now. A blank line MUST exist after the last line. This avoid ugly lines like #ifndef windows entry1. #endif #ifdef windows entry1.,\ entry2 #endif What happens if someone (inevitably) adds a new package to the list and forgets to do either of these? Does it result in a build failure? No build failure, but test/java/security/SecurityManager/CheckPackageAccess.java will fail. I can add check in the build tool. Otherwise looks good, although I think it would be useful to write an additional test to make sure the correct providers are installed and ordered correctly on the different platforms, something similar to the java/lang/SecurityManager/CheckPackageAccess.java test but specific to providers. OK. Thanks Max
RFR 6997010: Consolidate java.security files into one file with modifications
Please review the code change at http://cr.openjdk.java.net/~weijun/6997010/webrev.00/ The fix consolidates java.security-platform files into one with #ifdef directives. There are several major changes: 1. Creation of file is moved from CopyFiles to GenerateData, since we are really generating something now. Said that, the source data is kept in src/share/lib/security instead of make/data. I am OK with moving it if anyone desires. 2. The new tool MakeJavaSecurity includes the function of old AddToRestrictedPkgs. MakeJavaSecurity includes a new argument to deal with the platform dependent entries. The restricted.pkgs argument is also changed from a list of entries to a file name, so that we can also support the same #ifdef mechanism inside restricted.pkgs. 3. The new consolidated java.security supports #ifdef and #ifndef. It is not necessary to support #else or (and|or) of multiple #ifdef's now. 4. *IMPORTANT*: In order to easily maintain platform-related entries, every line (including the last line) in package.access and package.definition MUST end with ',\' now. A blank line MUST exist after the last line. This avoid ugly lines like #ifndef windows entry1. #endif #ifdef windows entry1.,\ entry2 #endif The MakeJavaSecurity tool will strip the trailing ,\ from the last line to make the file exactly the same as before, although personally I don't think it's really necessary since the following empty line will terminate the entry automatically. Thanks Max
