Issue Details (XML | Word | Printable)

Key: MPCHANGELOG-81
Type: Bug Bug
Status: Closed Closed
Resolution: Fixed
Priority: Major Major
Assignee: Unassigned
Reporter: Dennis Lundberg
Votes: 0
Watchers: 0
Operations

If you were logged in you would be able to see more operations.
Maven 1.x Changelog Plugin

Some valid scm urls are not allowed by this plugin

Created: 20/Jan/06 02:00 PM   Updated: 22/Jan/06 07:13 PM   Resolved: 22/Jan/06 07:13 PM
Return to search
Component/s: None
Affects Version/s: None
Fix Version/s: 1.9

Time Tracking:
Not Specified

File Attachments: 1. Java Source File ChangeLog2Test.java (3 kB)
2. Java Source File ChangeLogTest.java (4 kB)
3. Text File MPCHANGELOG-81-2.patch (7 kB)
4. Text File MPCHANGELOG-81.patch (4 kB)



 Description  « Hide

The solution to MPCHANGELOG-72 introduced a new bug. It is no longer possible to use an scm url that uses "|" as the separator.

There is also a possible future problem in this plugin with regards to scm urls. Upcoming versions of maven-scm will allow an scm url to start with "scm|" as well as "scm:".



Dennis Lundberg added a comment - 20/Jan/06 02:08 PM

Unfortunately this testcase is not complete. The class ChangeLog that I am trying to test is really hard to test because of its private methods. I am trying to test the method createFactory() but cannot access it directly as it is private.

Anyway, put this test file into a fresh checkout and run it. This will throw exceptions, because I have not managed to set up a correct environment to run the complete doExecute method in.

You will see that there are warnings for two of the tree tests, claiming that there is something wrong with the scm connections. The upcoming patch will fix both of these warnings.


Dennis Lundberg made changes - 20/Jan/06 02:08 PM
Field Original Value New Value
Attachment ChangeLogTest.java [ 18728 ]
Dennis Lundberg added a comment - 20/Jan/06 02:13 PM

This patch fixes the issues higlighted by the previous testcase. After this patch has been applied there should no long be any warnings when the previous testcase is run.


Dennis Lundberg made changes - 20/Jan/06 02:13 PM
Attachment MPCHANGELOG-81.patch [ 18729 ]
Lukas Theussl added a comment - 20/Jan/06 05:18 PM

Are you sure this is a valid connection string (from RepositoryTest.java):

scm:cvs|pserver|anoncvs@cvs.apache.org|/home/cvspublic|module

Shouldn't the separator be set to the character after the 'scm'?


Dennis Lundberg added a comment - 21/Jan/06 05:56 AM

That is a valid question. At the moment it does not have to be the same.
I ran this set of commands:
mvn scm:validate -DconnectionUrl="scm:cvs:pserver:anoncvs@cvs.apache.org:/home/cvspublic:module"
mvn scm:validate -DconnectionUrl="scm:cvs|pserver|anoncvs@cvs.apache.org|/home/cvspublic|module"
mvn scm:validate -DconnectionUrl="scm|cvs|pserver|anoncvs@cvs.apache.org|/home/cvspublic|module"

Using maven-scm-alpha-2 (couldn't find alpha-1 anywhere) produced these results:
VALID
VALID
INVALID

Using maven-scm-beta-3-SNAPSHOT produced these results:
VALID
VALID
VALID

As you can see this is consistent with what is in RepositoryTest. In both versions a url starting with "scm:" can use "|" as separator.

The only difference is in the third case, which I indicated is something that is coming in the next version of maven-scm.


Dennis Lundberg added a comment - 22/Jan/06 05:31 PM

Updated version of the test case that illuminates the problem. This is not to be checked in since it throws exceptions. When you run it you will receive one warning. That warning will disappear when the updated version of the patch is applied.


Dennis Lundberg made changes - 22/Jan/06 05:31 PM
Attachment ChangeLog2Test.java [ 18751 ]
Dennis Lundberg added a comment - 22/Jan/06 05:45 PM

Updated version of the patch. This patch affects a more files than the previous one. I had to tweak a few tests to successfully run "maven test".

There is one part of the code in RepositoryUtils.splitSCMConnection() that is not very pretty, but it get's the job done. I've added a comment in the code to explain what is going on there.


Dennis Lundberg made changes - 22/Jan/06 05:45 PM
Attachment MPCHANGELOG-81-2.patch [ 18752 ]
Lukas Theussl added a comment - 22/Jan/06 07:13 PM

Patch applied. Thanks!
For the record: we agreed on the scm-dev list that the format for a valid scm url should be scm:<provider><delimiter><provider-parameters>, ie it has to start with 'scm:', but <delimiter> can be either ':' or '|'.


Lukas Theussl made changes - 22/Jan/06 07:13 PM
Status Open [ 1 ] Closed [ 6 ]
Resolution Fixed [ 1 ]
Fix Version/s 1.9 [ 12040 ]