[NETBEANS-1936] Usage of remainderDelimiter

[gbrinkmann]

Usage of remainderDelimiter instead of filtering just "-dev" from a version number, for example when using SlikSvn 1.9.7-SlikSvn

[gbrinkmann]

Reference: https://issues.apache.org/jira/browse/NETBEANS-1936

[jlahoda]

So, I am not an expert on svn versioning, but my svn client says:

svn, version 1.9.7 (r1800392)

Looking at the code, the original code was able to parse that ("Version.parse(String)" automatically strips the second part after the space), but does the new version handle this? (And, is there time to introduce a test for this method?)

Thanks.

[gbrinkmann]

Hello,

the full version string from SlikSvn is svn, version 1.9.7-SlikSvn (SlikSvn/1.9.7)

The build I'm using is Apache NetBeans IDE 11.0 (Build incubator-netbeans-release-404-on-20190319)

The parse line (line 87) throws a NumberFormatException for input string "7-SlikSvn"

INFO [org.netbeans.modules.subversion]: Commandline client version: 1.9.7-SlikSvn (SlikSvn/1.9.7)
SEVERE [org.netbeans.modules.subversion]: For input string: "7-SlikSvn"
java.lang.NumberFormatException: For input string: "7-SlikSvn"
	at java.base/java.lang.NumberFormatException.forInputString(NumberFormatException.java:65)
	at java.base/java.lang.Integer.parseInt(Integer.java:652)
	at java.base/java.lang.Integer.parseInt(Integer.java:770)
	at org.netbeans.modules.subversion.client.cli.commands.VersionCommand$Version.parse(VersionCommand.java:168)
	at org.netbeans.modules.subversion.client.cli.commands.VersionCommand$Version.parse(VersionCommand.java:146)
	at org.netbeans.modules.subversion.client.cli.commands.VersionCommand.checkForErrors(VersionCommand.java:87)
	at org.netbeans.modules.subversion.client.cli.CommandlineClient.checkSupportedVersion(CommandlineClient.java:132)
	at org.netbeans.modules.subversion.client.SvnClientFactory.checkVersion(SvnClientFactory.java:473)
	at org.netbeans.modules.subversion.client.SvnClientFactory.checkCLIExecutable(SvnClientFactory.java:430)
	at org.netbeans.modules.subversion.client.SvnClientFactory.setupCommandline(SvnClientFactory.java:403)
	at org.netbeans.modules.subversion.client.SvnClientFactory.setup(SvnClientFactory.java:222)
	at org.netbeans.modules.subversion.client.SvnClientFactory.init(SvnClientFactory.java:95)
	at org.netbeans.modules.subversion.client.SvnClientFactory.isClientAvailable(SvnClientFactory.java:240)
	at org.netbeans.modules.subversion.Annotator.checkClientAvailable(Annotator.java:638)
	at org.netbeans.modules.subversion.Annotator.annotateIcon(Annotator.java:506)

The last change before mine just filters "-dev" from the version string, but the usage of the method signature Version.parse(String, String) is explicitely meant for truncating stuff like for example "-SNAPSHOT" from the version string, like stated in the javadoc.

I'll try to introduce a test.

[gbrinkmann]

Testcode: see #1248

[jlahoda]

I think I understand what you are trying to do, and that is fine.

But, your patch (appears to) break other usecases, specifically, parsing of versions like:
svn, version 1.9.7 (r1800392)

And that is not fine, IMO. So I am afraid you'll need to find a way to parse both/all the versions correctly.

[gbrinkmann]

Trying to find out why build 4 had problems connecting to maven (downloadbinaries) and/or how the test can be started again...

I see, according to https://stackoverflow.com/questions/17606874/trigger-a-travis-ci-rebuild-without-pushing-a-commit I cannot restart the build since I don't have write access.

[matthiasblaesing]

I tested your changeset on ubuntu and it breaks the default CLI client.

svn, Version 1.10.0 (r1827917)

before your change that was correctly parsed. I only had a quick lock at this, but maybe splitting with a regexp "[\D.]" would work.

[gbrinkmann]

Thanks to @matthiasblaesing and @jlahoda
I'll extend my test and optimize the fix.

Later:
I didn't change anything so far, because this method is intended to be used with strings beginning with a version number, so NumberFormatExceptions are expected if not.
See also the corresponding older test "testArguments".
And/or my corrected "testSplitting"

So the test would be "1.10.0 (r1827917)" or "1.9.7 (r1800392)" and this works fine (added to "testSplitting").

[matthiasblaesing]

Please have a look here:
https://github.com/matthiasblaesing/netbeans/tree/pr-1244
I added two commits to this. The problem is, that there are two variants of the parse method and the problematic combination is not tested. There is no sane reason for two parse methods as the class is purely internal, so I removed the second method and switched to regexp based splitting.

While I looked at your commits, I see that you are switching author information. In general a real name and correct email is expected. The email part looks clean in your case, the realname part switches between two pseudonyms and your realname. Please check your configuration.

For anyone merging this, this should be squashed at the end and the author information corrected.

[gbrinkmann]

Thanks again, I've applied your changes.
Are my credentials OK now? I've updated them locally and on github,

[matthiasblaesing]

Sorry, no your author information is still broken (look at the commit log with a client, that really shows you what is going on: git log -n 5. I'll see if the change looks sane now. If so, I'll squash, rebase and merge this.

[matthiasblaesing]

Merged as 0c15fc1

[gbrinkmann]

I guess this is because of committing from Netbeans 11 and not from the command line (git bash from gitforwindows).
Opening global configuration in Netbeans shows the correct values, I've set them before committing yesterday.
I'll play around and test. Thanks again for your continued support and for merging the pr.