RFR: 8035063: Option handling in sjavac needs to be rewritten
oehrstroem at gmail.com
Thu Mar 20 09:00:55 UTC 2014
Ok, Eriks explanation makes sense. There might also be situations where you
want to exclude/include copying META-INF files and such, that reside in
non-package directories. So, changing from . to / is ok.
In particular since sjavac cleverly detects and reports an error, if a
source file is in the wrong package directory.
See JavaCompilerWithDeps.java at the end, so there is already protection
against messing up the source tree. :-)
2014-03-19 17:19 GMT+01:00 Jonathan Gibbons <jonathan.gibbons at oracle.com>:
> I think sjavac should filter out all directories which could not
> correspond to package names.
> -- Jon
> On 03/19/2014 06:58 AM, Erik Joelsson wrote:
>> I should fill in that the background for this change is that there are a
>> bunch of directories named "doc-files" in the classes source dir. They have
>> been there a long time. These contain html and gif files (mainly) that
>> shouldn't get copied into the output classes directory.
>> In a recent cleanup of the copy mechanism, I changed the copy rules to by
>> default include html and gif files and explicitly exclude those that
>> shouldn't be included, rather than explicitly listing what should be
>> copied. Because of this, we now have the following exclude rules:
>> # These directories should not be copied at all
>> EXCLUDES += \
>> com/sun/org/apache/xml/internal/security/resource/schema \
>> java/awt/doc-files \
>> java/lang/doc-files \
>> javax/swing/doc-files \
>> javax/swing/text/doc-files \
>> javax/swing/plaf/synth/doc-files \
>> javax/swing/undo/doc-files \
>> sun/awt/X11/doc-files \
>> sun/util/cldr/resources \
>> When building with sjavac, these get translated to "-e
>> java.awt.doc-files.*" which sjavac does not accept, because doc-files are
>> not proper package names. Another possible fix would be to filter out non
>> valid package names from the excludes sent to sjavac.
>> On 2014-03-19 14:32, Andreas Lundblad wrote:
>>> On Wed, Mar 19, 2014 at 01:58:21PM +0100, Fredrik Öhrström wrote:
>>>> The code by itself looks good. However I think that changing from . to
>>>> / is
>>>> a really bad idea.
>>>> If someone committed a package directory that does not map correctly to
>>>> package name, then that is a major problem. You will not find that
>>>> through -sourcepath, which means that you just broke sjavac ability to
>>>> compile using multiple threads. This is just as bad as when Nashorn
>>>> to insert $ into java source file names! The horror, the horror.
>>>> I.e. do not replace . with /. Fix the problem in the jdk.
>>> In this case, the directory did not contain any source files, just a
>>> bunch of resources for documentation, such as .gif-files. (How could it
>>> contain source files? A package may not contain a '-'.) So unless I
>>> misunderstood you, there should therefore be no problem in this case.
>>> I would prefer to see -i and -x as filters on what directories (not
>>> packages) exists in the subsequent source root: This is more in line with
>>> other options (-if, -xf, -src, ...) which all talk about paths and more
>>> general since each package corresponds to a directory, but only some
>>> directories correspond to packages.
>>> - Andreas
More information about the build-dev