Fix broken git url detection

dsjense commented 2 years ago

Steps to reproduce

Git URLs such as git@host.com:path/to/repo.git are currently broken thanks to the filter in the function require_url_format in lib/spack/spack/util/url.py. The fix is pretty simple and amounts to replacing git:// with git| in that function. This is related to Issue https://github.com/spack/spack/pull/29765. I have a branch with the change at https://github.com/dsjense/spack/tree/bugfix/allow_git_urls. We probably should add a unit test though so that this bug doesn't sneak in again.

I'm mainly interested in using URLs of the form git@host.com:path/to/repo.git for my private repositories but this bug is easy to reproduce by taking any code with github URLs and switching them to use the git@github.com syntax. For example, I put the following in the faust package to trigger the bug:

    git      = "git@github.com:grame-cncm/faust.git"
    version('2.27.2', commit='5c45bff8f7e83969426e7036b186b7715c3ecda0')

Just to emphasize, there is nothing wrong with the faust package. I was just looking for a package with few dependencies that I could modify to trigger this bug.

Error message

Traceback (most recent call last):
  File "/mnt/development/myspack/spack/bin/spack", line 98, in <module>
    sys.exit(spack.main.main())
  File "/mnt/development/myspack/spack/lib/spack/spack/main.py", line 893, in main
    return _main(argv)
  File "/mnt/development/myspack/spack/lib/spack/spack/main.py", line 848, in _main
    return finish_parse_and_run(parser, cmd_name, env_format_error)
  File "/mnt/development/myspack/spack/lib/spack/spack/main.py", line 876, in finish_parse_and_run
    return _invoke_command(command, parser, args, unknown)
  File "/mnt/development/myspack/spack/lib/spack/spack/main.py", line 533, in _invoke_command
    return_val = command(parser, args)
  File "/mnt/development/myspack/spack/lib/spack/spack/cmd/install.py", line 437, in install
    install_specs(args, kwargs, zip(abstract_specs, specs))
  File "/mnt/development/myspack/spack/lib/spack/spack/cmd/install.py", line 263, in install_specs
    builder.install()
  File "/mnt/development/myspack/spack/lib/spack/spack/installer.py", line 1648, in install
    self._prepare_for_install(task)
  File "/mnt/development/myspack/spack/lib/spack/spack/installer.py", line 915, in _prepare_for_install
    if restage and task.pkg.stage.managed_by_spack:
  File "/mnt/development/myspack/spack/lib/spack/spack/package_base.py", line 1127, in stage
    self._stage = self._make_stage()
  File "/mnt/development/myspack/spack/lib/spack/spack/package_base.py", line 1104, in _make_stage
    stage = self._make_root_stage(fetcher)
  File "/mnt/development/myspack/spack/lib/spack/spack/package_base.py", line 1081, in _make_root_stage
    self.spec)
  File "/mnt/development/myspack/spack/lib/spack/spack/mirror.py", line 407, in mirror_archive_paths
    global_ref = fetcher.mirror_id()
  File "/mnt/development/myspack/spack/lib/spack/spack/fetch_strategy.py", line 854, in mirror_id
    repo_path = url_util.parse(self.url).path
  File "/mnt/development/myspack/spack/lib/spack/spack/util/url.py", line 79, in parse
    require_url_format(url)
  File "/mnt/development/myspack/spack/lib/spack/spack/util/url.py", line 348, in require_url_format
    raise ValueError('Invalid url format from url: %s' % url)
ValueError: Invalid url format from url: git@github.com:grame-cncm/faust.git

Information on your system

Spack: 0.19.0.dev0 (bbbf95bf6a14f4595ad913e21d2f4dcc81a8036d)
Python: 3.6.9
Platform: linux-ubuntu18.04-skylake
Concretizer: clingo

General information

[X] I have run spack debug report and reported the version of Spack/Python/Platform
[X] I have searched the issues of this repo and believe this is not a duplicate
[X] I have run the failing commands in debug mode and reported the output

dsjense commented 2 years ago

This appears to be a duplicate of https://github.com/spack/spack/pull/29674. I think the documentation should probably be updated to indicate that we have to use the ssh://git@bitbucket.org/name/repo.git syntax or else merge @certik's fix.

certik commented 2 years ago

I think my fix should still be merged, so many people keep hitting this issue. But I don't know how to run tests locally (see #29674), so I don't know how to bring it over the finish line.

dsjense commented 2 years ago

@certik I ran the test locally and it failed for me. Some debugging shows that the url_util.parse function thinks git@... URL syntax uses the file scheme:

parsed=ParseResult(scheme='file', netloc='', path='/mnt/development/spack_contrib/certik/spack/git@bitbucket.org:name/repo.git', params='', query='', fragment=None)

Changing the test to the following lets it pass but still doesn't look right.

    parsed = url_util.parse('git@github.com:spack/spack.git', scheme='git')
    assert(parsed.scheme == 'git')
    assert(parsed.netloc == '')
    assert(parsed.path == 'git@github.com:spack/spack.git')

I was expecting the code to somehow access the parse_git_url function but it never does. It looks like it tries URLFetchStrategy in fetch_strategy.py and then switches to GitFetchStrategy. The URLFetchStrategy methods call url_util.parse (leading to the failure in require_url_format that needs fixing) before trying to clone the repo with GitFetchStrategy methods. It makes me wonder what parse_git_url is even used for.

certik commented 2 years ago

@dsjense thanks! The actual fix is trivial, so it might be worth just merging without testing, if it is hard to test for.

spack / spack