Hervé BOUTEMY wrote:
Le mercredi 20 juin 2007, Kenney Westerhof a écrit :
Also see
https://svn.apache.org/repos/asf/maven/surefire/trunk/maven-surefire-plugin
/src/main/java/org/apache/maven/plugin/surefire/SurefirePlugin.java
line 494.
I tried to fix this a while back but it's a big mess and has a lot of
unwanted side effects.
Accessors shouldn't change a property, but this one does because only when
you really need the version, it's resolved (lazy init). Though I don't
understand why X-SNAPSHOT needs to be resolved to the timestamped version
to determine wheter it is a snapshot..
btw, baseversion is the version containing the SNAPSHOT keyword,
version is the one containing the timestamp-buildernumber IIRC.
ok,
so I think that if a transformation has to be done from xxx-timestamp into
xxx-SNAPHOT, then it should be in the setBaseVersion() method, so the
baseVersion attribute is directly set to xxx-SNAPSHOT and never changed
after, even if the parameter during the call was xxx-timestamp.
Is this ok to do so, or should we investigate why it is called sometimes with
a timestamped version and not -SNAPSHOT?
Well, just looked at the code and it's a mess. Here are the problems:
* The isSnapshot method does a regexp on the version for the timestamp, and if
it matches,
replaces the baseversion with -SNAPSHOT. If it doesn't match, checks for
'endswith(SNAPSHOT)'.
So this method changes the baseversion on some occasions.
* The above means that calling isSnapshot on an artifact with baseversion
'1SNAPSHOT' or 'SNAPSHOT' is
considered a snapshot IFF the baseversion is not timestamp-resolved yet.
So, it'll return true for non-timestamp resolved artifacts, but
it'll return false if they have. This is because the regexp checks for ^(.*)-(\d\d\detc....)',
so the timestamp has to be preceded by '-'.
* LATEST is also considered a snapshot, if the version has not been resolved.
If it has been resolved
to a real version (1.0 for instance), it won't be a snapshot. I'm not sure if
this is a problem,
if only snapshots are considered in resolution. Shouldn't LATEST resolve to
non-snapshots?
Ah, they do: AbstractVersionTransformation:
// Don't use snapshot metadata for LATEST (which isSnapshot returns true
for)
if ( !artifact.isSnapshot() || Artifact.LATEST_VERSION.equals(
artifact.getBaseVersion() ) )
{
metadata = new ArtifactRepositoryMetadata( artifact );
}
else
{
metadata = new SnapshotArtifactRepositoryMetadata( artifact );
}
This needs to be fixed - i'm sure there's a reason for LATEST being treated
like snapshots at some
places, and not at later places. As soon as LATEST is resolved it's no longer
a snapshot, surely this
causes problems?
* There are multiple methods for version modification/retrieval:
* resolved version (setter only - sets version)
* version (sets version, baseversion, and versionrange; gets version)
* base version (sets base version, gets baseversion or version if baseversion
null)
* version range (sets versionrange, and sets version and baseversion either to
null or a recommeded version; gets versionrange)
* available versions
* selectVersion (sets version and baseversion)
* updateVersion (sets version and file)
These can be set independently and can result in an inconsistent state.
Documentation lacks to describe
which you can and which you cannot call in what sequences.
* isRelease is a boolean that has nothing to do with versions. I'd figure that
if something wasn't a snapshot,
it'd be a release. Guess not. Or maybe it is, but then the logic is someplace
else.
Simplifying this should make a whole lot of difference.
For now, we should update the setters to immediately update baseversion to
-SNAPSHOT,
so that even when you don't call isSNapshot, getBaseVersion is ok. Just
updating setBaseVersion isn't enough,
but it's a start. If you do setBaseVersion(timestamp) it'll discard the real
value and replace it with -SNAPSHOT.
Ideally I'd like to NOT use the regexps to find out if something is a snapshot,
but use the repository
metadata for that. Maybe some company uses timestamps for releases, who knows..
-- Kenney
Hervé
-- Kenney
Hervé BOUTEMY wrote:
Le mercredi 20 juin 2007, Brian E. Fox a écrit :
I discovered http://jira.codehaus.org/browse/MNG-2961 while working on
mdep a while back.
same for me while working on http://jira.codehaus.org/browse/MANTTASKS-18
It should be an easy fix but I'm pondering what the
least intrusive fix is.
thought it would be easy too, but as you mention, it must be the least
intrusive: that's the hard part.
Clearly calling a Boolean method shouldn't
result in a change to some other variable so we should do something.
Changing it to always returns xxx-SNAPSHOT as it does after calling
isSnapshot has the potential to break things depending on existing
behavior. The cleanest way forward is most likely to not have isSnapshot
modify the version, and to create a new method that returns xxx-SNAPSHOT
and leave getBaseVersion to return just the xxx-timestamp. (not sure
what this method would be called...getNonResolvedBaseVersion)
WDYT?
creating a new method is intrusive since this new method has to be used
now: in this case, I know of
o.a.m.artifact.repository.layout.DefaultRepositoryLayout.pathOf(), but I
suppose there are others.
To avoid a change in the API, I was thinking at modifying setVersion()
and setBaseVersion() to do the job actually done in isSnapshot(). But I
don't really understand the semantics of setBaseVersion(): as you noted,
this has the potential to break things.
To make a good decision, I think the first thing is to clearly describe
the semantics of version and baseVersion: perhaps it is already clearly
explained somewhere, but I don't know where. Depending the result, we'll
be able to decide if the API has to be extended, or only the
implementation fixed (and assume if it breaks something that works now
but shouldn't because baseVersion had been used where version should
have).
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
