Hi,
On 11/14/2018 09:52 AM, Andy Herrick wrote:
On 11/13/2018 5:50 PM, Philip Race wrote:
On 11/13/18, 12:52 PM, Roger Riggs wrote:
Hi,
There are enough files unique to each platform to put them in
separate packages
otherwise you get too many (IMHO) files in a single
package/directory and
its harder to tell which go with which. There isn't much of a
problem with
classes being public because they are all in a module and not exported.
I would put them all under
share/classes/jdk/jpackagers/internal/<OS> and
save a directory level.
That isn't how the rest of the JDK is organised.
Platform specific classes are split where you have "share" in the
path above.
So whilst the other issues are more arguable I don't think the build
team
would like platform specific classes under share. There is already an
objection to that for the native files.
To the "too many files in one package/directory" point.
I think that might happen at the directory level if Andy went through
with
his suggestion but I don't think it will happen with what I proposed and
we probably should get some benefit from being able to make classes +
methods
package private.
but my proposal only increases the number of files in each directory
as follows:
share/classes/jdk/jpackager/internal - from 28 to 31
windows/classes/jdk/jpackager/internal - from 6 to 7
macosx/classes/jdk/jpackager/internal - from 6 to 7
linux/classes/jdk/jpackager/internal - from 3 to 4
(adding Main.java, that makes are only 50 source files total)
so I hardly see any any impact of having "too many files in one
package/directory"
and for Phil's part of the change, although the platform dependent
source files would be in the same package as the platform independent
files, they are still in a different source dir, and every platform
specific source file's name starts with the platform ("Win", "Mac", or
"Linux")
This should be fine, since there is no need to for cross-platform support,
the reason for the top level platform directory is so the build can be
selectively
include the right files.
I assume the service loader mechanism deals with all the issues in
locating platform specific files.
finally a question about resources:
Each source file that references resources has it's own resource file
(23 resource files in each of 3 languages for a total of 69 properties
files).
That seems like overkill to me. One result is duplicate resources
whenever a key is referenced in more than one source file.
Wouldn't it simplify things to combine these into a smaller set of
resource files ?
agreed, there should be fewer resource files to track and maintain and
NO duplication.
Roger
/Andy
So I think what I proposed is about right ..
phil
$.02, Roger
On 11/13/2018 03:46 PM, Andy Herrick wrote:
I agree with this and would take it further.
1 file is in ./share/classes/jdk/jpackager/internal/builders - why
not just ./share/classes/jdk/jpackager/internal
2 files are in ./share/classes/jdk/jpackager/internal/bundlers -
why not just in ./share/classes/jdk/jpackager/internal
1 file is in ./linux/classes/jdk/jpackager/internal/builders/linux
- why not just ./linux/classes/jdk/jpackager/internal
1 file is in ./macosx/classes/jdk/jpackager/internal/builders/mac -
why not just ./macosx/classes/jdk/jpackager/internal
1 file is in
./windows/classes/jdk/jpackager/internal/builders/windows - why not
just ./windows/classes/jdk/jpackager/internal
then just move the associated resources -
Basically put everything except Main in same package - everything
would be easier to find, and we could make almost all methods
package-private (the only exception would be the few things called
by Main, and the ToolProvider.
On 11/13/2018 2:54 PM, Phil Race wrote:
Question .. why is "mac", "linux" and "windows" necessary in the
package name here ?
src/jdk.jpackager/macosx/classes/jdk/jpackager/internal/mac/MacAppBundler.java
src/jdk.jpackager/windows/classes/jdk/jpackager/internal/builders/windows/WindowsAppImageBuilder.java
src/jdk.jpackager/linux/classes/jdk/jpackager/internal/linux/LinuxRpmBundler.java
There's not likely to be a clash, so is there some other reason
not to want these
in the same package as the shared internals like
src/jdk.jpackager/share/classes/jdk/jpackager/internal/Param.java
?
-phil.
