Page MenuHomeSoftware Heritage

add support for the CVS loader to 'Save Code Now'
ClosedPublic

Authored by stsp on Jul 13 2021, 3:42 PM.

Diff Detail

Repository
rDWAPPS Web applications
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 23915
Build 37297: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 37296: arc lint + arc unit

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Jul 13 2021, 3:43 PM
Harbormaster failed remote builds in B22596: Diff 21624!

add support for the CVS loader to 'Save Code Now'

Build has FAILED

Patch application report for D5992 (id=21628)

Rebasing onto 4a9cf1d618...

Current branch diff-target is up to date.
Changes applied before test
commit 19dca75c0ff7f215fe9bc74b9c74e86873fef75d
Author: Stefan Sperling <stsp@stsp.name>
Date:   Tue Jul 13 15:41:18 2021 +0200

    add support for the CVS loader to 'Save Code Now'

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/966/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/966/console

Harbormaster returned this revision to the author for changes because remote builds failed.Jul 13 2021, 4:40 PM
Harbormaster failed remote builds in B22600: Diff 21628!

add support for the CVS loader to 'Save Code Now'

Build has FAILED

Patch application report for D5992 (id=21791)

Rebasing onto 481647f863...

Current branch diff-target is up to date.
Changes applied before test
commit 953d420949a8c49bccc4e878e593ca8a83086ce3
Author: Stefan Sperling <stsp@stsp.name>
Date:   Tue Jul 13 15:41:18 2021 +0200

    add support for the CVS loader to 'Save Code Now'

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/984/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/984/console

Harbormaster returned this revision to the author for changes because remote builds failed.Jul 27 2021, 3:01 PM
Harbormaster failed remote builds in B22759: Diff 21791!
  • add support for the CVS loader to 'Save Code Now'
  • sort list of visit types alphabetically

Build is green

Patch application report for D5992 (id=22241)

Rebasing onto 66865c37ce...

Current branch diff-target is up to date.
Changes applied before test
commit 253bf73c0e657938961dac71124005ffde17728a
Author: Stefan Sperling <stsp@stsp.name>
Date:   Thu Aug 26 14:02:17 2021 +0200

    sort list of visit types alphabetically

commit e25e91d9b101378fbd07f681285548e946384f9e
Author: Stefan Sperling <stsp@stsp.name>
Date:   Tue Jul 13 15:41:18 2021 +0200

    add support for the CVS loader to 'Save Code Now'

See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1036/ for more details.

stsp requested review of this revision.Aug 26 2021, 2:18 PM
ardumont added a subscriber: ardumont.

lgtm

Thanks.

Don't forget to add yourself to the top-level CONTRIBUTORS file ;)

This revision is now accepted and ready to land.Sep 1 2021, 9:25 AM

Also I just realized, as it's not completely ready afaik, don't land it yet though.

@anlambert What do you think of this?

Do we want for example to have a configuration (file) option to hide it and we toggle it later, when it's ready?
That'd avoid to let this diff hang.

TIA

Also I just realized, as it's not completely ready afaik, don't land it yet though.

@anlambert What do you think of this?

Do we want for example to have a configuration (file) option to hide it and we toggle it later, when it's ready?
That'd avoid to let this diff hang.

TIA

Instead of relying on a configuration option, we could check if the load-cvs task type is present in the scheduler database
and activate the save code now for cvs based on that result.

Currently it is not, see below:

print(
    [
        t["type"]
        for t in swhweb_config["scheduler"].get_task_types()
        if "load" in t["type"]
    ]
)
['load-hg', 'load-svn', 'load-npm', 'load-pypi', 'load-git', 'load-svn-from-archive', 'load-hg-from-archive', 'load-git-from-dir', 'load-git-from-zip', 'load-nixguix', 'load-deb-package', 'load-deposit', 'load-archive-files', 'load-svn-from-remote-dump', 'load-cran']

What do you think ?

What do you think ?

yes, good idea. That'd simplify the setup both in docker and prod (as in nothing to do ;)

What do you think ?

yes, good idea. That'd simplify the setup both in docker and prod (as in nothing to do ;)

D6170

@stsp , thanks for this !

I am putting that diff in changes requested mode as I made some changes in swh-web (D6170, D6184)
to allow the addition of a new visit type to 'Save code now' even if the associated loader is not yet available
in production.

It will allow you to test save code now for cvs in docker environment while ensuring the cvs visit type will
be available in production only when its loader deployed.

Please find below your diff adapted once rebased on latest swh-web HEAD.

diff --git a/assets/src/bundles/save/index.js b/assets/src/bundles/save/index.js
index d2c54ac7..1a893b32 100644
--- a/assets/src/bundles/save/index.js
+++ b/assets/src/bundles/save/index.js
@@ -358,7 +358,7 @@ export function validateSaveOriginUrl(input) {
   }
 
   if (validUrl) {
-    const allowedProtocols = ['http:', 'https:', 'svn:', 'git:'];
+    const allowedProtocols = ['http:', 'https:', 'svn:', 'git:', 'rsync:', 'pserver:', 'ssh:'];
     validUrl = (
       allowedProtocols.find(protocol => protocol === originUrl.protocol) !== undefined
     );
diff --git a/cypress/integration/origin-save.spec.js b/cypress/integration/origin-save.spec.js
index 0c245eb0..47dc1c84 100644
--- a/cypress/integration/origin-save.spec.js
+++ b/cypress/integration/origin-save.spec.js
@@ -19,8 +19,8 @@ const saveCodeMsg = {
   'csrfError': 'CSRF Failed: Referrer checking failed - no Referrer.'
 };
 
-const anonymousVisitTypes = ['git', 'hg', 'svn'];
-const allVisitTypes = ['archives', 'git', 'hg', 'svn'];
+const anonymousVisitTypes = ['cvs', 'git', 'hg', 'svn'];
+const allVisitTypes = ['archives', 'cvs', 'git', 'hg', 'svn'];
 
 function makeOriginSaveRequest(originType, originUrl) {
   cy.get('#swh-input-origin-url')
diff --git a/swh/web/common/origin_save.py b/swh/web/common/origin_save.py
index d088cb26..62d74e81 100644
--- a/swh/web/common/origin_save.py
+++ b/swh/web/common/origin_save.py
@@ -121,7 +121,12 @@ def can_save_origin(origin_url: str, bypass_pending_review: bool = False) -> str
 
 # map visit type to scheduler task
 # TODO: do not hardcode the task name here (T1157)
-_visit_type_task = {"git": "load-git", "hg": "load-hg", "svn": "load-svn"}
+_visit_type_task = {
+    "git": "load-git",
+    "hg": "load-hg",
+    "svn": "load-svn",
+    "cvs": "load-cvs",
+}
 
 _visit_type_task_privileged = {
     "archives": "load-archive-files",
@@ -203,7 +208,9 @@ def _check_visit_type_savable(visit_type: str, privileged_user: bool = False) ->
         )
 
 
-_validate_url = URLValidator(schemes=["http", "https", "svn", "git"])
+_validate_url = URLValidator(
+    schemes=["http", "https", "svn", "git", "rsync", "pserver", "ssh"]
+)
 
 
 def _check_origin_url_valid(origin_url: str) -> None:
diff --git a/swh/web/config.py b/swh/web/config.py
index dba7378f..a78c7434 100644
--- a/swh/web/config.py
+++ b/swh/web/config.py
@@ -34,6 +34,7 @@ ORIGIN_VISIT_TYPES = [
     "pypi",
     "svn",
     "tar",
+    "cvs",
 ]
 
 
diff --git a/swh/web/templates/misc/origin-save.html b/swh/web/templates/misc/origin-save.html
index 60fa8d72..7d676721 100644
--- a/swh/web/templates/misc/origin-save.html
+++ b/swh/web/templates/misc/origin-save.html
@@ -85,6 +85,9 @@ See top-level LICENSE file for more information
           <li><code>git</code>, for origins using <a href="https://git-scm.com/">Git</a></li>
           <li><code>hg</code>, for origins using <a href="https://www.mercurial-scm.org/">Mercurial</a></li>
           <li><code>svn</code>, for origins using <a href="https://subversion.apache.org/">Subversion</a></li>
+          {% if "cvs" in visit_types %}
+            <li><code>cvs</code>, for origins using <a href="http://cvs.nongnu.org/">CVS</a></li>
+          {% endif %}
         </ul>
       </li>
       <li><b>Origin url:</b> the url of the remote repository for the software origin.<br/>
diff --git a/swh/web/tests/conftest.py b/swh/web/tests/conftest.py
index fead2e30..751d2476 100644
--- a/swh/web/tests/conftest.py
+++ b/swh/web/tests/conftest.py
@@ -457,6 +457,21 @@ def swh_scheduler(swh_scheduler):
             "retry_delay": timedelta(hours=2),
         }
     )
+    # create load-cvs task type
+    swh_scheduler.create_task_type(
+        {
+            "type": "load-cvs",
+            "description": "Update a CVS repository",
+            "backend_name": "swh.loader.cvs.tasks.LoadCVS",
+            "default_interval": timedelta(days=64),
+            "min_interval": timedelta(hours=12),
+            "max_interval": timedelta(days=64),
+            "backoff_factor": 2,
+            "max_queue_length": None,
+            "num_retries": 7,
+            "retry_delay": timedelta(hours=2),
+        }
+    )
 
     # add method to add load-archive-files task type during tests
     def add_load_archive_task_type():
diff --git a/swh/web/tests/misc/test_origin_save.py b/swh/web/tests/misc/test_origin_save.py
index b357a138..bd9561d2 100644
--- a/swh/web/tests/misc/test_origin_save.py
+++ b/swh/web/tests/misc/test_origin_save.py
@@ -14,7 +14,7 @@ from swh.web.common.origin_save import SAVE_REQUEST_ACCEPTED, SAVE_TASK_SUCCEEDE
 from swh.web.common.utils import reverse
 from swh.web.tests.utils import check_http_get_response
 
-VISIT_TYPES = ("git", "svn", "hg")
+VISIT_TYPES = ("git", "svn", "hg", "cvs")
 PRIVILEGED_VISIT_TYPES = tuple(list(VISIT_TYPES) + ["archives"])
This revision now requires changes to proceed.Sep 3 2021, 4:24 PM

Build has FAILED

Patch application report for D5992 (id=22960)

Rebasing onto 9e51595ad7...

Current branch diff-target is up to date.
Changes applied before test
commit e66e206750f05250b2fbaac8fa5c6de0cabc5053
Author: Stefan Sperling <stsp@stsp.name>
Date:   Tue Jul 13 15:41:18 2021 +0200

    add support for the CVS loader to 'Save Code Now'

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1113/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1113/console

include changes suggested by anlambert which I missed

Build is green

Patch application report for D5992 (id=22981)

Rebasing onto 8aa625784d...

First, rewinding head to replay your work on top of it...
Applying: add support for the CVS loader to 'Save Code Now'
Changes applied before test
commit cfcc003ef664ba4fb51f6b58b74de9e204cac394
Author: Stefan Sperling <stsp@stsp.name>
Date:   Tue Jul 13 15:41:18 2021 +0200

    add support for the CVS loader to 'Save Code Now'
    
    Includes changes from anlambert suggested during code review.

See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1114/ for more details.

Looks good to me, just a small indentation issue to fix before landing this. Thanks !

swh/web/templates/misc/origin-save.html
87–91

indentation is not correct here, these should be:

<li><code>svn</code>, for origins using <a href="https://subversion.apache.org/">Subversion</a></li>
{% if "cvs" in visit_types %}
  <li><code>cvs</code>, for origins using <a href="http://cvs.nongnu.org/">CVS</a></li>
{% endif %}
swh/web/tests/conftest.py
449

thanks !

This revision is now accepted and ready to land.Sep 22 2021, 6:22 PM
stsp marked an inline comment as done.Sep 22 2021, 7:17 PM
stsp added inline comments.
swh/web/templates/misc/origin-save.html
87–91

This was a tabs vs. spaces issue. Will fix it.

stsp marked an inline comment as done.

whitespace fix

Build is green

Patch application report for D5992 (id=22987)

Rebasing onto 8aa625784d...

First, rewinding head to replay your work on top of it...
Applying: add support for the CVS loader to 'Save Code Now'
Changes applied before test
commit b25af5e89cf0f00bc1dc062bfd9cf98a2f9bc0c3
Author: Stefan Sperling <stsp@stsp.name>
Date:   Tue Jul 13 15:41:18 2021 +0200

    add support for the CVS loader to 'Save Code Now'
    
    Includes changes from anlambert suggested during code review.

See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1115/ for more details.

Build is green

Patch application report for D5992 (id=22989)

Rebasing onto 8aa625784d...

Current branch diff-target is up to date.
Changes applied before test
commit 6722da987bd4c1eb5124ee2d6eebcbeca1ca0fde
Author: Stefan Sperling <stsp@stsp.name>
Date:   Tue Jul 13 15:41:18 2021 +0200

    add support for the CVS loader to 'Save Code Now'
    
    Includes changes from anlambert suggested during code review.

See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1116/ for more details.