RFR: 8035063: Option handling in sjavac needs to be rewritten

[Fredrik Öhrström]

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. :-)

//Fredrik



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.
>>
>> /Erik
>>
>> 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
>>>> the
>>>> package name, then that is a major problem. You will not find that
>>>> source
>>>> through -sourcepath, which means that you just broke sjavac ability to
>>>> compile using multiple threads. This is just as bad as when Nashorn
>>>> tried
>>>> 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
>>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20140320/e6b2e1dc/attachment.html>