Defect #14422

CVS root_url not recognized when connection string does not include port

Added by Dominik Follmann over 4 years ago. Updated over 4 years ago.

Status:ClosedStart date:
Priority:NormalDue date:
Assignee:Jean-Philippe Lang% Done:

0%

Category:SCM
Target version:2.3.3
Resolution:Fixed Affected version:

Description

The patch fixes a bug in the CVS SCM module of the standard version of redmine (2.3.1).
Without this bugfix, the revisions are not handled correctly. Each revision-number (e.g. 1.4) is only accepted once by the redmine repository, although each file has its own revision-numbers.

In our case the CVSROOT looks like this:

:pserver:cvs_user:cvs_password@123.456.789.123:9876/repo

The problem is located in \lib\redmine\scm\adapters\cvs_adapter.rb in the method root_url_path
--> RegExp was changed from
/^:.+:\d*/
to
/^(:.+)+@\d+(.\d+)+/

This lets the property return the correct part of the CVSROOT-URL
("/repo" instead of "::9876/repo")
And now the function 'fetch_changesets' works as expected with CVS.

Find attached the patch file.

cvs_adapter.rb.patch Magnifier (246 Bytes) Dominik Follmann, 2013-07-10 08:39

cvs_adapter.rb (1).patch Magnifier (246 Bytes) tyrel cropper, 2013-07-26 05:01

Associated revisions

Revision 12017
Added by Jean-Philippe Lang over 4 years ago

Adds a test for CvsAdapter#root_url_path (#14422).

Revision 12027
Added by Jean-Philippe Lang over 4 years ago

CVS root_url not recognized when connection string does not include port (#14422).

Revision 12065
Added by Jean-Philippe Lang over 4 years ago

Merged r12017 and r12027 from trunk (#14422).

History

#1 Updated by Toshi MARUYAMA over 4 years ago

  • Target version set to 2.3.2

#2 Updated by Toshi MARUYAMA over 4 years ago

  • Target version changed from 2.3.2 to 2.4.0

#3 Updated by Jean-Philippe Lang over 4 years ago

  • Status changed from New to Needs feedback

Test added in r12017, it passes without your proposed fix. Would you have a failing test?

#4 Updated by Dominik Follmann over 4 years ago

Good morning.
Sorry, I was wrong in my description and copy-pasted the wrong CVSROOT. This happened, because I already put wrong comments to my source code when I implemented the patch some weeks ago ;-)
What I meant was:

:pserver:cvs_user:cvs_password@123.456.789.123/repo

So without the explicit port after the IP-Address.
So the correct test would be:
      def test_root_url_path
        adapter = Redmine::Scm::Adapters::CvsAdapter.new('foo', ':pserver:cvs_user:cvs_password@123.456.789.123/repo')
        assert_equal '/repo', adapter.send(:root_url_path)
      end

Regards

#5 Updated by Jean-Philippe Lang over 4 years ago

  • Tracker changed from Patch to Defect
  • Subject changed from Improvement for CVS-Adapter to handle CVSROOT correct to CVS root_url not recognized when connection string does not include port
  • Status changed from Needs feedback to Resolved
  • Assignee set to Jean-Philippe Lang
  • Target version changed from 2.4.0 to 2.3.3
  • Resolution set to Fixed

The proposed patch matches an IP adress only. It won't work if a host name is used.
A different fix is committed in r12027, thanks for pointing this out.

#6 Updated by Dominik Follmann over 4 years ago

Hey, you're right, I forgot the hostname possibility.Thank you for the improvement!

#8 Updated by Jean-Philippe Lang over 4 years ago

  • Status changed from Resolved to Closed

Merged.

Also available in: Atom PDF