You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ant.apache.org by Stefan Bodewig <bo...@apache.org> on 2003/02/07 11:07:39 UTC
API changes introduced while fixing bug 10755
I'll summarize what has happened in this mail and send a different one
on issues I have myself with the changes.
I ask you to take this summary as starting points to actually review
the code.
(1) New data-type Resource
this is an abstraction of File and ZipEntry (for now) that is used to
handle <zipfileset> and <fileset> in a uniform way inside <zip>.
More importantly it is needed to adapt the old SourceFileScanner logic
in a way that the target is not a file, but a ZipEntry. This change
is crucial to fix 10755 as we need a way to compare timestamps of the
files to add with the corresponding ZipEntries in the target archive.
(2) New interface ResourceFactory
implemented by DirectoryScanner and ZipScanner for now. A simple way
to get the Resource corresponding to a name (as returned by a
FileNameMapper).
(3) New interface ResourceScanner that adds getIncludedFileResources
and getIncludedDirectoryResources to FileScanner.
implemented by DirectoryScanner and overridden in ZipScanner.
(4) New utility class SourceSelector.
This is basically SourceFileScanner extended to the Resource concept,
chosing some better names on the way.
Contains a lot of code moved over from SourceFileScanner.
(5) SourceFileScanner now delegates to SourceSelector.
This is basically a boackwards compatibility step. This also means
that bugs introduced via the Resource concept will affect all tasks
using SourceFileScanner.
(6) ZipScanner now really parses the archive instead of returning the
archive itself in getIncludedFiles.
It has only ever been used in <zip>, and here it is needed to get the
fix right. It breaks backwards compatibility and will be marked as
such in WHATSNEW.
(7) Zip has more or less been completely rewritten.
It breaks API backwards compatibility and will be marked as such in
WHATSNEW.
(8) Jar, War and Ear have been adapted to changes in Zip.
It breaks API backwards compatibility and will be marked as such in
WHATSNEW.
Stefan
Re: API changes introduced while fixing bug 10755
Posted by Stefan Bodewig <bo...@apache.org>.
On Wed, 12 Feb 2003, Antoine Levy-Lambert <le...@tiscali-dsl.de>
wrote:
> What is the benefit of TaskLogger in terms of design, compared to
> the current situation of having a reference to the current task in
> ZipScanner ?
You wouldn't get access to the rest of the Task interface inside the
Scanner.
> Don't we need a static method or a "factory" somewhere that all
> types could use to find the official logger for the current task ?
Brian talked about his approach. You can get access to the Task that
belongs to the current thread as long as you have access to the
project. Nothing static AFAIK.
> re removing the ResourceScanner interface
>
> Fine with me.
Done 8-)
Stefan
Re: API changes introduced while fixing bug 10755
Posted by Stefan Bodewig <bo...@apache.org>.
On Thu, 13 Feb 2003, Bruce Atherton <br...@callenish.com> wrote:
> Personally, I'd like to see a completely different use of Scanners
> in the system.
+1
Actually I would have loved it to be that way in 1.2 - but
MatchingTask and backwards compatibility prevented it. What I regret
now is that we didn't provide a different way to use filesets at the
same time so that new tasks could have used the "better" way.
Stefan
Re: API changes introduced while fixing bug 10755
Posted by Antoine Levy-Lambert <le...@tiscali-dsl.de>.
Thanks for your comments Bruce.
Antoine Lévy-Lambert
----- Original Message -----
From: "Bruce Atherton" <br...@callenish.com>
To: "Ant Developers List" <de...@ant.apache.org>
Sent: Thursday, February 13, 2003 10:52 PM
Subject: Re: API changes introduced while fixing bug 10755
> At 02:28 AM 2/12/2003, Antoine Levy-Lambert wrote:
>
> >4) for the future
> >
> >I think we should change a number of method signatures to use Resource
> >instead of File.
>
> s/change/add/ (and mark the old methods deprecated), at least for methods
> already in a release. Until we decide it is time to clean out the cruft
and
> release Ant 2.
>
>
> >I have the following in mind :
> > - adding new properties to the Resource class
>
> The eventual plan would be to make this Resource class represent an entry
> in a virtual file system. Therefore, it should provide either a subset of
> all common properties on the different file systems (not desirable as it
is
> far too limiting) or a superset of properties with sensible behaviour when
> a property is used on a file system that doesn't provide it. Whether that
> behaviour is a BuildException, a warning message, or some default
behaviour
> would depend on the specific circumstances. So, for example, attempting to
> get the compressed size of a file on disk could just return the regular
> size of the file.
>
>
> > - create a new routine in DirectoryScanner called Resource []
> >ListDir(Resource base)
>
> Personally, I'd like to see a completely different use of Scanners in the
> system. Conceptually, I don't think Tasks should work with Scanners
> directly, but instead get everything they need from fileset
implementations
> of AbstractFileSet (AbstractResourceSet?). Scanners should be command
> objects that populate the required fields of the fileset. Whether the
> contents of the fileset come from a scanner or the hand of god is not
> something a task should be concerned about.
>
> Eventually. And for the foreseeable future, the old way has to continue to
> work as well.
>
>
> >it might ... pave the way for
> >doing new things, like using a ZipFileSet as a source in the <copy> task
>
> Or an FTPFileSet, or a DAVFileSet, or a CVSRevisionSet, or ...
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
> For additional commands, e-mail: dev-help@ant.apache.org
>
Re: API changes introduced while fixing bug 10755
Posted by Bruce Atherton <br...@callenish.com>.
At 02:28 AM 2/12/2003, Antoine Levy-Lambert wrote:
>4) for the future
>
>I think we should change a number of method signatures to use Resource
>instead of File.
s/change/add/ (and mark the old methods deprecated), at least for methods
already in a release. Until we decide it is time to clean out the cruft and
release Ant 2.
>I have the following in mind :
> - adding new properties to the Resource class
The eventual plan would be to make this Resource class represent an entry
in a virtual file system. Therefore, it should provide either a subset of
all common properties on the different file systems (not desirable as it is
far too limiting) or a superset of properties with sensible behaviour when
a property is used on a file system that doesn't provide it. Whether that
behaviour is a BuildException, a warning message, or some default behaviour
would depend on the specific circumstances. So, for example, attempting to
get the compressed size of a file on disk could just return the regular
size of the file.
> - create a new routine in DirectoryScanner called Resource []
>ListDir(Resource base)
Personally, I'd like to see a completely different use of Scanners in the
system. Conceptually, I don't think Tasks should work with Scanners
directly, but instead get everything they need from fileset implementations
of AbstractFileSet (AbstractResourceSet?). Scanners should be command
objects that populate the required fields of the fileset. Whether the
contents of the fileset come from a scanner or the hand of god is not
something a task should be concerned about.
Eventually. And for the foreseeable future, the old way has to continue to
work as well.
>it might ... pave the way for
>doing new things, like using a ZipFileSet as a source in the <copy> task
Or an FTPFileSet, or a DAVFileSet, or a CVSRevisionSet, or ...
Re: API changes introduced while fixing bug 10755
Posted by Antoine Levy-Lambert <le...@tiscali-dsl.de>.
Stefan, Bruce,
1) logging in Scanners
Stefan Bodewig wrote :
> True. I could as well pass in an org.apache.tools.ant.util.TaskLogger
> instance.
>
What is the benefit of TaskLogger in terms of design, compared to the
current situation of having a reference to the current task in ZipScanner ?
Don't we need a static method or a "factory" somewhere that all types could
use to find the official logger for the current task ?
2) exceptions
Bruce Atherton wrote :
>As someone who hates checked exceptions, I'm probably the wrong person to
>talk to this point. I'll just point out that BuildExceptions already
>percolate throughout the code without necessarily being declared.
I agree that it would be better to throw BuildExceptions rather than log
errors or print them on System.err. I did not know that BuildException(s)
did not have to be declared in the method signatures.
3) ResourceScanner interface
re removing the ResourceScanner interface
>
>I'm not convinced that this is really needed (at this point).
Fine with me. It looks like ResourceScanner will be then defacto replaced by
DirectoryScanner. Or do you have something else in mind ?
4) for the future
I think we should change a number of method signatures to use Resource
instead of File.
I have the following in mind :
- adding new properties to the Resource class
size to record file sizes for selectors
symlink to record whether a Resource is a simlink because there
simlinks are scanned with a special treatment in DirectoryScanner
an object or several properties to know the "base URL" of the
resource,
ie for a zip entry the path of the zip file,
for a file in a fileset the basedir,
- changing the BaseSelector API so that isSelected only takes as
argument
we would have :
public boolean isSelected(Resource someresource)
instead of
public boolean isSelected(File basedir, String filename, File file)
- create a new routine in DirectoryScanner called Resource []
ListDir(Resource base)
this routine would be also implemented (but differently) in all
subclasses of DirectoryScanner
it would give the list of all entries which are directly under base,
without recursion.
and would be called from DirectoryScanner#scan
then ideally, subclasses of DirectoryScanner would not need their own
implementation of scan, but would just rely on ListDir
This is probably a lot of work and a revolution in some areas of the ant
APIs, but it might disentangle a number of routines and pave the way for
doing new things, like using a ZipFileSet as a source in the <copy> task,
....
Comments ?
Antoine
Re: API changes introduced while fixing bug 10755
Posted by Stefan Bodewig <bo...@apache.org>.
On Tue, 11 Feb 2003, Bruce Atherton <br...@callenish.com> wrote:
> This is in large part a conversation about the colour of a bikeshed,
8-)
> ResourceUtils would still work,
I can go with that, no problem.
> OTOH, perhaps the methods in SelectorUtils fit within the contract
> of SourceSelector and should be folded into it, perhaps with a
> broadening of the types of parameters that are accepted.
I'll look into it.
> Eventually we'd like a future where Scanners can be used
> interchangeably in all tasks, no?
Probably yes, it would jump out of the polymorphism ideas in a way.
>> > Is it necessary that Scanners call the logging API?
>>
>>Not really. System.err would probably work. The problem are debug
>>messages that you normally would want to suppress, we can as well
>>drop them.
>
> Regarding System.err, that seems unnecessary to me if you are
> willing to throw BuildExceptions (unless I'm missing something).
BuildExceptions for sever errors, System.err for what we'd log with
MSG_ERR priority, System.out for MSG_INFO was the idea.
> then logging probably is a better solution than System.err even if
> it complicates the API. It gives more control to our logging
> infrastructure.
True. I could as well pass in an org.apache.tools.ant.util.TaskLogger
instance.
> As for debug messages, the question is whether you need access to
> the Scanner internals for debugging, or if all you need is
> information about the external behaviour (which can be logged at a
> higher level in the program).
There currently are debug level messages that need access to the
internals, I'm not sure how much valuable information they'd provide
though.
Stefan
Re: API changes introduced while fixing bug 10755
Posted by Bruce Atherton <br...@callenish.com>.
At 06:33 AM 2/11/2003, Stefan Bodewig wrote:
>On Fri, 07 Feb 2003, Bruce Atherton <br...@callenish.com> wrote:
> > If the comment at the top of the file is a true indication of what
> > it is for, then how about "ResourceProcessingUtils", or perhaps more
> > simply "ResourceUtils".
>
>s/process/select/ in the comment. What about the name then?
This is in large part a conversation about the colour of a bikeshed, but
since you ask...
We seem to have a loosely adhered to policy of calling a class that
provides static methods xxxUtils, so even though this is in the utils
directory I think it is a good idea to include that suffix.
For better or worse, the term "selector" has a meaning in Ant, and I'm not
a big fan of overloading meanings if we don't have to. With the
substitution in the comment, perhaps ResourceSelectionUtils would be a
better name (although still potentially confusing). ResourceUtils would
still work, too, making this class the resource for all static utility
methods that work with resources.
OTOH, perhaps the methods in SelectorUtils fit within the contract of
SourceSelector and should be folded into it, perhaps with a broadening of
the types of parameters that are accepted. Eventually we'd like a future
where Scanners can be used interchangeably in all tasks, no? You could
declare the class is for all utility methods that support filtering a
collection of resources. Even then, I'd go with a name like
ResourceSelectorUtils or something similar. "Source" is misleading in the
name of this class, I think.
> > Is it necessary that Scanners call the logging API?
>
>Not really. System.err would probably work. The problem are debug
>messages that you normally would want to suppress, we can as well drop
>them.
Regarding System.err, that seems unnecessary to me if you are willing to
throw BuildExceptions (unless I'm missing something). If the current code
prohibits declaring BuildException in the method signature and you aren't
willing to use exceptions without compile-time checks (you can always
document them in the Javadoc), then logging probably is a better solution
than System.err even if it complicates the API. It gives more control to
our logging infrastructure.
As for debug messages, the question is whether you need access to the
Scanner internals for debugging, or if all you need is information about
the external behaviour (which can be logged at a higher level in the
program). The tradeoff is between getting more details about low level
operations and having looser coupling between these low level pieces and
the rest of the system. My inclination is for looser coupling unless people
developing custom tasks are likely to need those details. Just a
preference, though.
So my thinking (and what I did in Selectors) is to avoid logging anything
at that level, provide methods if necessary that allow referees to get
information for debug messages if they need it (eg. toString), and throw
BuildExceptions with abandon.
Just my POV.
Re: API changes introduced while fixing bug 10755
Posted by Stefan Bodewig <bo...@apache.org>.
On Fri, 07 Feb 2003, Bruce Atherton <br...@callenish.com> wrote:
> At 02:28 AM 2/7/2003, Stefan Bodewig wrote:
>
>>One thing that I might want to see some stricter contract enforced
>>is the name. getName() will currently return names using the
>>platform's file separator as separator for Files and forward slashes
>>for ZipEntryies.
>>
>>I'd prefer to make that always use something predictable, and the
>>platform's separator may be the better choice.
>
> Depending on what you mean by "platform",
In this case, the OS. So it would be \ on DOS based systems and / on
Unix. The same abstraction File.separator uses. I have no idea what
it is set to on OpenVMS.
> I think this would would not be the best choice since it makes the
> VFS layer you talked about more complicated to eventually do.
So the alternative is "always use /". Fine with me, makes the code in
ZipScanner even easier to write.
>> > (3) New interface ResourceScanner
>>
>>I'm not convinced that this is really needed (at this point).
>
> I have to agree.
So let's axe it.
>> > (4) New utility class SourceSelector.
>> >
>> > chosing some better names on the way.
>>
>>At least I hope so. 8-)
>
> If the comment at the top of the file is a true indication of what
> it is for, then how about "ResourceProcessingUtils", or perhaps more
> simply "ResourceUtils".
s/process/select/ in the comment. What about the name then?
> Is it necessary that Scanners call the logging API?
Not really. System.err would probably work. The problem are debug
messages that you normally would want to suppress, we can as well drop
them.
Stefan
Re: API changes introduced while fixing bug 10755
Posted by Bruce Atherton <br...@callenish.com>.
At 02:28 AM 2/7/2003, Stefan Bodewig wrote:
>I am aware that this could lead to a VFS layer and has a lot of
>potential for future expansions, but for now it has been kept as a
>bare minimum.
>
>One thing that I might want to see some stricter contract enforced is
>the name. getName() will currently return names using the platform's
>file separator as separator for Files and forward slashes for
>ZipEntryies.
>
>I'd prefer to make that always use something predictable, and the
>platform's separator may be the better choice.
Depending on what you mean by "platform", I think this would would not be
the best choice since it makes the VFS layer you talked about more
complicated to eventually do. Would you want URLs, for example, to have
forward slashes converted to a platform's separators?
If consistency is needed, my preference would be to define a canonical
"Resource" path separator that works on the majority of potential
filesystems (aka '/'). This may add a bit more complication up-front, but
it has the virtue that it will still work in the future we envisage for
ourselves. The downside is that on some platforms it will involve a lot
more conversion back and forth.
> > (3) New interface ResourceScanner
>
>I'm not convinced that this is really needed (at this point).
I have to agree. I'm not sure I see much benefit to an interface like this
until we go for a fully abstracted file system.
> > (4) New utility class SourceSelector.
> >
> > chosing some better names on the way.
>
>At least I hope so. 8-)
If the comment at the top of the file is a true indication of what it is
for, then how about "ResourceProcessingUtils", or perhaps more simply
"ResourceUtils".
>I have an issue with the implementation, not with the API.
>
>ZipScanner now wants a Task instance to log to. This should probably
>be a ProjectComponent so that ZipFileSet can point to itself in
>getDirectoryScanner. Or maybe it should get the Project instance
>directly?
Is it necessary that Scanners call the logging API? Couldn't the layer
around them do that, relying on the information provided through toString()
and BuildExceptions?
>Furthermore, errors get just logged, I'd prefer to really bail out and
>throw a BuildException here. The problem is that these are going to happen in
>getIncluded*() methods, that are not declared to throw anything (I
>know BuildException is not a checked exception, but it doesn't look
>right to throw them without telling the user).
As someone who hates checked exceptions, I'm probably the wrong person to
talk to this point. I'll just point out that BuildExceptions already
percolate throughout the code without necessarily being declared.
Re: API changes introduced while fixing bug 10755
Posted by Stefan Bodewig <bo...@bost.de>.
On 07 Feb 2003, Stefan Bodewig <bo...@apache.org> wrote:
> One thing that I might want to see some stricter contract enforced
> is the name.
Done, mostly via javadocs. Based on Bruce's suggestion it will ow
always use / as separator.
>> (3) New interface ResourceScanner
>
> I'm not convinced that this is really needed (at this point).
Has been removed.
>> (4) New utility class SourceSelector.
>>
>> chosing some better names on the way.
>
> At least I hope so. 8-)
I've been convinced that either ResourceUtils or SelectorUtils would
be better - I'll look into the later (as it already exists) to see how
it fits.
> ZipScanner now wants a Task instance to log to.
Removed. I've removed all debug output (which may make development a
slight bit more difficult) and turned the logged errors into
exceptions.
Stefan
Re: API changes introduced while fixing bug 10755
Posted by Bill Burton <bi...@progress.com>.
Hello,
Stefan Bodewig wrote:
> On 07 Feb 2003, Stefan Bodewig <bo...@apache.org> wrote:
>>(6) ZipScanner now really parses the archive instead of returning
>>the archive itself in getIncludedFiles.
>
>
> I have an issue with the implementation, not with the API.
>
> ZipScanner now wants a Task instance to log to. This should probably
> be a ProjectComponent so that ZipFileSet can point to itself in
> getDirectoryScanner. Or maybe it should get the Project instance
> directly?
I'm writing a generic Velocity to Ant logging adapter initially to use
for http://vpp.sourceforge.net. I just ran into a similar issue when
logging from a filter.
Since filters must be a subclass of java.io.FilterReader, they can't
also extend ProjectComponent. As a result, they only have access to the
Project, not the owning task. Logging through the Project is not good
because the output is not associated with the calling Task.
After much head scratching and looking around, I eventually found this
seems to work:
LogSystem logger = null;
Task owningTask =
getProject().getThreadTask(Thread.currentThread());
if (owningTask != null) {
logger = new AntLogSystem(owningTask);
}
else {
logger = new AntLogSystem(getProject());
}
So if you're willing to pass Project into ZipScanner, it could log
against the owning Task.
-Bill
Re: API changes introduced while fixing bug 10755
Posted by Stefan Bodewig <bo...@apache.org>.
On 07 Feb 2003, Stefan Bodewig <bo...@apache.org> wrote:
> and send a different one on issues I have myself with the changes.
here it is.
> (1) New data-type Resource
I am aware that this could lead to a VFS layer and has a lot of
potential for future expansions, but for now it has been kept as a
bare minimum.
One thing that I might want to see some stricter contract enforced is
the name. getName() will currently return names using the platform's
file separator as separator for Files and forward slashes for
ZipEntryies.
I'd prefer to make that always use something predictable, and the
platform's separator may be the better choice.
> (2) New interface ResourceFactory
related to the above: what does the factory expect as argument to
getResource method. I'd like to tighten the contract.
Currently it will accept names using \ or / and do the right thing
(implicitly as java.io.File does for Files and explicitly in
ZipScanner for ZipEntry). I'd like to make that a requirement along
the lines of "be open in what you get and strict in what you return"
or so.
> (3) New interface ResourceScanner
I'm not convinced that this is really needed (at this point).
If you look at the implenentation in DirectoryScanner, the new methods
are a mere convenience layer on top of getIncludedFiles/Directories.
The same is true for the implementation in ZipScanner, even if it is
less visibly so.
This may be useful at some future point if we wanted to extend other
tasks to work on Resource objects instead of filenames.
> (4) New utility class SourceSelector.
>
> chosing some better names on the way.
At least I hope so. 8-)
Also the names should help us if we want to use different selection
mechanisms in the future.
> (6) ZipScanner now really parses the archive instead of returning
> the archive itself in getIncludedFiles.
I have an issue with the implementation, not with the API.
ZipScanner now wants a Task instance to log to. This should probably
be a ProjectComponent so that ZipFileSet can point to itself in
getDirectoryScanner. Or maybe it should get the Project instance
directly?
Furthermore, errors get just logged, I'd prefer to really bail out and
throw a BuildException here. The problem is that these are going to happen in
getIncluded*() methods, that are not declared to throw anything (I
know BuildException is not a checked exception, but it doesn't look
right to throw them without telling the user).
Stefan