Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding ability to compare git references to spack install #24639

Merged
merged 65 commits into from
Sep 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
65 commits
Select commit Hold shift + click to select a range
090c798
adding ability to compare git references to spack install
vsoch Jun 30, 2021
3a23a34
Merge branch 'develop' into add/github-version-comparison
vsoch Jul 9, 2021
ed4a3c8
fixing isort issues
vsoch Jul 9, 2021
7e7cf9f
use pkg.git over pkg.url for git fetches
becker33 Jul 20, 2021
be4a574
ensuring that we strip .git from a repository url, otherwise the outp…
vsoch Jul 20, 2021
d2a9101
ensure we do not compare versions if either the primary or other vers…
vsoch Jul 20, 2021
dd09212
we cannot check for commit versions if the package does not have a gi…
vsoch Jul 21, 2021
3f14ff7
cannot use rstrip as it also strips other letters in the string, usin…
vsoch Jul 21, 2021
20534e5
generate commit lookups for dev specs when added to concretization
becker33 Jul 21, 2021
534b7a6
type safety for Version.__lt__ and Version.satisfies
becker33 Jul 21, 2021
58e9b10
user other.commits if it exists and self.commits does not
becker33 Jul 21, 2021
6b8be68
fall back on prev_version in Version.__contains__
becker33 Jul 21, 2021
7ee3d79
fix undefined variable
becker33 Jul 22, 2021
60692c0
move version lookup to post-parse pass
becker33 Jul 22, 2021
364ab4b
add line in fetch_strategy.py for flake
becker33 Jul 22, 2021
7d0795f
adding test for clone by command and supporting filesystem clone
vsoch Jul 22, 2021
ed3e43d
style fixes
vsoch Jul 22, 2021
e9d08a5
ensure we tell git who we are
vsoch Jul 22, 2021
7281bfb
Adding comment about generating commit lookup
vsoch Aug 24, 2021
0534420
small updates to fetch strategy docs and version
vsoch Aug 24, 2021
92dcbd8
invalid syntax?
vsoch Aug 24, 2021
f19ec90
updating logic in commit parsing
vsoch Aug 24, 2021
cddea2e
commits is never used
vsoch Aug 24, 2021
1acb19a
Can git repo change to have >= 5 branches?
vsoch Aug 25, 2021
10c59cd
refactor clone method signature
becker33 Aug 25, 2021
344f88c
do not require that a fetcher only has one branch, tag, or commit
vsoch Aug 25, 2021
71fe3eb
Make sure that git log gets --all commits
vsoch Aug 29, 2021
ed81ede
try using tuples with empty string delimiter
becker33 Aug 30, 2021
e70e542
flake
becker33 Aug 30, 2021
6e110b1
typo bugfix
becker33 Aug 30, 2021
7f0d7ba
fixup test for more robust git repo
becker33 Sep 2, 2021
8a40af7
make git commit install test stricter
becker33 Sep 2, 2021
b4d58a3
test for comparing git versions
becker33 Sep 2, 2021
b217616
minor fixes to git version lookups
becker33 Sep 2, 2021
a832244
flake
becker33 Sep 10, 2021
a4557e3
isort
becker33 Sep 10, 2021
a93bb56
more isort
becker33 Sep 10, 2021
ea48f30
fix isort/flake8 issues
tgamblin Sep 10, 2021
023e34e
refactor genreate_commit_lookup to instantiate its own fetcher
becker33 Sep 13, 2021
1eff677
refactor git is-ancestor check to avoid try/except
becker33 Sep 13, 2021
3edccca
refactor to avoid one git request per tag
becker33 Sep 13, 2021
2967a3d
fix version comparisons to tags
becker33 Sep 13, 2021
3e135a4
os.makedirs -> llnl.util.filesystem.mkdirp
becker33 Sep 13, 2021
e48f568
remove unreachable code block
becker33 Sep 13, 2021
28a52ca
CommitLookup: refactor to lazy cache initialization for performance
becker33 Sep 13, 2021
3f4ac09
improve error message and docstring
becker33 Sep 13, 2021
8b5fc5b
CommitLookup: use misc cache for locking
becker33 Sep 14, 2021
b9a0431
Merge branch 'add/github-version-comparison' of github.com:vsoch/spac…
becker33 Sep 14, 2021
28f9f19
flake
becker33 Sep 14, 2021
7960364
bugfix: type(distance) must be int
becker33 Sep 14, 2021
0ffaf68
ensure mock git version repo has distinct commit timestamps
becker33 Sep 14, 2021
60f5705
docstring for mock_git_version_info
becker33 Sep 14, 2021
5c7329a
remove vestigial code from PR
becker33 Sep 14, 2021
7f10b80
improved docstrings and error messages
becker33 Sep 14, 2021
fee95c5
add `spack.util.url.git_url_parse()` method and tests
tgamblin Sep 14, 2021
4280784
Merge branch 'add/github-version-comparison' of github.com:vsoch/spac…
becker33 Sep 14, 2021
7d7d550
refactor git parsing to make use of git_url_parse
becker33 Sep 14, 2021
a254d39
fix formatting for pytest fixture docstring
becker33 Sep 14, 2021
16fe132
fix import cycle
becker33 Sep 14, 2021
526d6f7
use bare clones for caching git repos
becker33 Sep 14, 2021
17963d2
Merge branch 'develop' into add/github-version-comparison
becker33 Sep 14, 2021
f8accd4
flake
becker33 Sep 14, 2021
d6bfa79
isort
becker33 Sep 14, 2021
2d98944
git_url_parse: fix docstring syntax error
becker33 Sep 14, 2021
b6214e1
minor bugfix
becker33 Sep 14, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
87 changes: 61 additions & 26 deletions lib/spack/spack/fetch_strategy.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,10 @@
import spack.util.pattern as pattern
import spack.util.url as url_util
import spack.util.web as web_util
import spack.version
from spack.util.compression import decompressor_for, extension
from spack.util.executable import CommandNotFoundError, which
from spack.util.string import comma_and, quote
from spack.version import Version, ver

#: List of all fetch strategies, created by FetchStrategy metaclass.
all_strategies = []
Expand Down Expand Up @@ -750,7 +750,7 @@ def __init__(self, **kwargs):
@property
def go_version(self):
vstring = self.go('version', output=str).split(' ')[2]
return Version(vstring)
return spack.version.Version(vstring)

@property
def go(self):
Expand Down Expand Up @@ -843,7 +843,7 @@ def version_from_git(git_exe):
"""
version_output = git_exe('--version', output=str)
m = re.search(GitFetchStrategy.git_version_re, version_output)
return Version(m.group(1))
return spack.version.Version(m.group(1))

@property
def git(self):
Expand All @@ -852,7 +852,7 @@ def git(self):

# Disable advice for a quieter fetch
# https://github.com/git/git/blob/master/Documentation/RelNotes/1.7.2.txt
if self.git_version >= Version('1.7.2'):
if self.git_version >= spack.version.Version('1.7.2'):
self._git.add_default_arg('-c')
self._git.add_default_arg('advice.detachedHead=false')

Expand Down Expand Up @@ -895,44 +895,71 @@ def fetch(self):
tty.debug('Already fetched {0}'.format(self.stage.source_path))
return

self.clone(commit=self.commit, branch=self.branch, tag=self.tag)

def clone(self, dest=None, commit=None, branch=None, tag=None, bare=False):
"""
Clone a repository to a path.

This method handles cloning from git, but does not require a stage.

Arguments:
dest (str or None): The path into which the code is cloned. If None,
requires a stage and uses the stage's source path.
commit (str or None): A commit to fetch from the remote. Only one of
commit, branch, and tag may be non-None.
branch (str or None): A branch to fetch from the remote.
tag (str or None): A tag to fetch from the remote.
bare (bool): Execute a "bare" git clone (--bare option to git)
"""
tgamblin marked this conversation as resolved.
Show resolved Hide resolved
# Default to spack source path
dest = dest or self.stage.source_path
tty.debug('Cloning git repository: {0}'.format(self._repo_info()))

git = self.git
if self.commit:
debug = spack.config.get('config:debug')

if bare:
# We don't need to worry about which commit/branch/tag is checked out
clone_args = ['clone', '--bare']
if not debug:
clone_args.append('--quiet')
clone_args.extend([self.url, dest])
git(*clone_args)
elif commit:
# Need to do a regular clone and check out everything if
# they asked for a particular commit.
debug = spack.config.get('config:debug')

clone_args = ['clone', self.url]
if not debug:
clone_args.insert(1, '--quiet')
with temp_cwd():
git(*clone_args)
repo_name = get_single_file('.')
self.stage.srcdir = repo_name
shutil.move(repo_name, self.stage.source_path)
if self.stage:
self.stage.srcdir = repo_name
shutil.move(repo_name, dest)
tgamblin marked this conversation as resolved.
Show resolved Hide resolved

with working_dir(self.stage.source_path):
checkout_args = ['checkout', self.commit]
with working_dir(dest):
checkout_args = ['checkout', commit]
if not debug:
checkout_args.insert(1, '--quiet')
git(*checkout_args)

else:
# Can be more efficient if not checking out a specific commit.
args = ['clone']
if not spack.config.get('config:debug'):
if not debug:
args.append('--quiet')

# If we want a particular branch ask for it.
if self.branch:
args.extend(['--branch', self.branch])
elif self.tag and self.git_version >= ver('1.8.5.2'):
args.extend(['--branch', self.tag])
if branch:
args.extend(['--branch', branch])
elif tag and self.git_version >= spack.version.ver('1.8.5.2'):
args.extend(['--branch', tag])

# Try to be efficient if we're using a new enough git.
# This checks out only one branch's history
if self.git_version >= ver('1.7.10'):
if self.git_version >= spack.version.ver('1.7.10'):
if self.get_full_repo:
args.append('--no-single-branch')
else:
Expand All @@ -942,22 +969,23 @@ def fetch(self):
# Yet more efficiency: only download a 1-commit deep
# tree, if the in-use git and protocol permit it.
if (not self.get_full_repo) and \
self.git_version >= ver('1.7.1') and \
self.git_version >= spack.version.ver('1.7.1') and \
self.protocol_supports_shallow_clone():
args.extend(['--depth', '1'])

args.extend([self.url])
git(*args)

repo_name = get_single_file('.')
self.stage.srcdir = repo_name
shutil.move(repo_name, self.stage.source_path)
if self.stage:
self.stage.srcdir = repo_name
shutil.move(repo_name, dest)
tgamblin marked this conversation as resolved.
Show resolved Hide resolved

with working_dir(self.stage.source_path):
with working_dir(dest):
# For tags, be conservative and check them out AFTER
# cloning. Later git versions can do this with clone
# --branch, but older ones fail.
if self.tag and self.git_version < ver('1.8.5.2'):
if tag and self.git_version < spack.version.ver('1.8.5.2'):
# pull --tags returns a "special" error code of 1 in
# older versions that we have to ignore.
# see: https://github.com/git/git/commit/19d122b
Expand All @@ -971,7 +999,7 @@ def fetch(self):
git(*co_args)

if self.submodules_delete:
with working_dir(self.stage.source_path):
with working_dir(dest):
for submodule_to_delete in self.submodules_delete:
args = ['rm', submodule_to_delete]
if not spack.config.get('config:debug'):
Expand All @@ -980,7 +1008,7 @@ def fetch(self):

# Init submodules if the user asked for them.
if self.submodules:
with working_dir(self.stage.source_path):
with working_dir(dest):
args = ['submodule', 'update', '--init', '--recursive']
if not spack.config.get('config:debug'):
args.insert(1, '--quiet')
Expand Down Expand Up @@ -1502,8 +1530,15 @@ def for_package_version(pkg, version):

check_pkg_attributes(pkg)

if not isinstance(version, Version):
version = Version(version)
if not isinstance(version, spack.version.Version):
version = spack.version.Version(version)

# if it's a commit, we must use a GitFetchStrategy
if version.is_commit and hasattr(pkg, "git"):
# Populate the version with comparisons to other commits
version.generate_commit_lookup(pkg)
fetcher = GitFetchStrategy(git=pkg.git, commit=str(version))
return fetcher

# If it's not a known version, try to extrapolate one by URL
if version not in pkg.versions:
Expand Down
3 changes: 3 additions & 0 deletions lib/spack/spack/paths.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@
reports_path = os.path.join(user_config_path, "reports")
monitor_path = os.path.join(reports_path, "monitor")

# We cache repositories (git) in first, extracted metadata in second
user_repos_cache_path = os.path.join(user_config_path, 'git_repos')

opt_path = os.path.join(prefix, "opt")
etc_path = os.path.join(prefix, "etc")
system_etc_path = '/etc'
Expand Down
9 changes: 9 additions & 0 deletions lib/spack/spack/spec.py
Original file line number Diff line number Diff line change
Expand Up @@ -4699,6 +4699,15 @@ def do_parse(self):
except spack.parse.ParseError as e:
raise SpecParseError(e)

# Generate lookups for git-commit-based versions
for spec in specs:
tgamblin marked this conversation as resolved.
Show resolved Hide resolved
# Cannot do lookups for versions in anonymous specs
# Only allow concrete versions using git for now
if spec.name and spec.versions.concrete and spec.version.is_commit:
pkg = spec.package
if hasattr(pkg, 'git'):
spec.version.generate_commit_lookup(pkg)

return specs

def spec_from_file(self):
Expand Down
29 changes: 28 additions & 1 deletion lib/spack/spack/test/cmd/install.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import spack.environment as ev
import spack.hash_types as ht
import spack.package
import spack.util.executable
from spack.error import SpackError
from spack.main import SpackCommand
from spack.spec import CompilerSpec, Spec
Expand Down Expand Up @@ -224,7 +225,7 @@ def test_install_overwrite(


def test_install_overwrite_not_installed(
mock_packages, mock_archive, mock_fetch, config, install_mockery
mock_packages, mock_archive, mock_fetch, config, install_mockery,
):
# Try to install a spec and then to reinstall it.
spec = Spec('libdwarf')
Expand All @@ -236,6 +237,32 @@ def test_install_overwrite_not_installed(
assert os.path.exists(spec.prefix)


def test_install_commit(
mock_git_version_info, install_mockery, mock_packages, monkeypatch):
"""
Test installing a git package from a commit.

This ensures Spack appropriately associates commit versions with their
packages in time to do version lookups. Details of version lookup tested elsewhere
"""
repo_path, filename, commits = mock_git_version_info
monkeypatch.setattr(spack.package.PackageBase,
'git', 'file://%s' % repo_path,
raising=False)

commit = commits[-1]
spec = spack.spec.Spec('git-test-commit@%s' % commit)
spec.concretize()
spec.package.do_install()

# Ensure first commit file contents were written
installed = os.listdir(spec.prefix.bin)
assert filename in installed
with open(spec.prefix.bin.join(filename), 'r') as f:
content = f.read().strip()
assert content == '[]' # contents are weird for another test


def test_install_overwrite_multiple(
mock_packages, mock_archive, mock_fetch, config, install_mockery
):
Expand Down
97 changes: 97 additions & 0 deletions lib/spack/spack/test/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,103 @@ def last_two_git_commits(scope='session'):
yield regex.findall(git_log_out)


def write_file(filename, contents):
with open(filename, 'w') as f:
f.write(contents)


commit_counter = 0


@pytest.fixture
def mock_git_version_info(tmpdir, scope="function"):
"""Create a mock git repo with known structure

The structure of commits in this repo is as follows::

| o fourth 1.x commit (1.2)
| o third 1.x commit
| |
o | fourth main commit (v2.0)
o | third main commit
| |
| o second 1.x commit (v1.1)
| o first 1.x commit
| /
|/
o second commit (v1.0)
o first commit

The repo consists of a single file, in which the Version._cmp representation
of each commit is expressed as a string.

Important attributes of the repo for test coverage are: multiple branches,
version tags on multiple branches, and version order is not equal to time
order or topological order.
"""
git = spack.util.executable.which('git', required=True)
repo_path = str(tmpdir.mkdir('git_repo'))
filename = 'file.txt'

def commit(message):
global commit_counter
git('commit', '--date', '2020-01-%02d 12:0:00 +0300' % commit_counter,
'-am', message)
commit_counter += 1

with working_dir(repo_path):
tgamblin marked this conversation as resolved.
Show resolved Hide resolved
git("init")

git('config', 'user.name', 'Spack')
git('config', 'user.email', 'spack@spack.io')

# Add two commits on main branch
write_file(filename, '[]')
git('add', filename)
commit('first commit')

# Get name of default branch (differs by git version)
main = git('rev-parse', '--abbrev-ref', 'HEAD', output=str, error=str).strip()

# Tag second commit as v1.0
write_file(filename, "[1, 0]")
commit('second commit')
git('tag', 'v1.0')

# Add two commits and a tag on 1.x branch
git('checkout', '-b', '1.x')
write_file(filename, "[1, 0, '', 1]")
commit('first 1.x commit')

write_file(filename, "[1, 1]")
commit('second 1.x commit')
git('tag', 'v1.1')

# Add two commits and a tag on main branch
git('checkout', main)
write_file(filename, "[1, 0, '', 1]")
commit('third main commit')
write_file(filename, "[2, 0]")
commit('fourth main commit')
git('tag', 'v2.0')

# Add two more commits on 1.x branch to ensure we aren't cheating by using time
tgamblin marked this conversation as resolved.
Show resolved Hide resolved
git('checkout', '1.x')
write_file(filename, "[1, 1, '', 1]")
commit('third 1.x commit')
write_file(filename, "[1, 2]")
commit('fourth 1.x commit')
git('tag', '1.2') # test robust parsing to different syntax, no v

# Get the commits in topo order
log = git('log', '--all', '--pretty=format:%H', '--topo-order',
output=str, error=str)
commits = [c for c in log.split('\n') if c]

# Return the git directory to install, the filename used, and the commits
yield repo_path, filename, commits


@pytest.fixture(autouse=True)
def clear_recorded_monkeypatches():
yield
Expand Down
2 changes: 1 addition & 1 deletion lib/spack/spack/test/git_fetch.py
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ def test_get_full_repo(get_full_repo, git_version, mock_git_repository,
ncommits = len(commits)

if get_full_repo:
assert(nbranches == 5)
assert(nbranches >= 5)
assert(ncommits == 2)
else:
assert(nbranches == 2)
Expand Down