From a044d5218d8cf1aca8920aa46ddb528b5517a03e Mon Sep 17 00:00:00 2001 From: Ben Fitzpatrick Date: Wed, 18 Oct 2023 20:12:23 +0100 Subject: [PATCH 01/12] The start --- .github/workflows/test.yml | 3 + metomi/rose/config_processors/fileinstall.py | 7 +- metomi/rose/loc_handlers/fs.py | 2 +- metomi/rose/loc_handlers/git.py | 191 ++++++++++++++++ metomi/rose/loc_handlers/namelist.py | 2 +- metomi/rose/loc_handlers/rsync.py | 2 +- sphinx/api/configuration/file-creation.rst | 13 ++ sphinx/installation.rst | 3 + t/rose-app-run/05-file.t | 15 +- t/rose-app-run/06-namelist.t | 4 +- t/rose-app-run/15-file-permission.t | 2 +- t/rose-app-run/28-git.t | 227 +++++++++++++++++++ 12 files changed, 460 insertions(+), 11 deletions(-) create mode 100644 metomi/rose/loc_handlers/git.py create mode 100644 t/rose-app-run/28-git.t diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index b7faba5a71..8cd10b03e5 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -131,6 +131,9 @@ jobs: __HERE__ cat "$HOME/.bashrc" + - name: Configure git # Configure Git for Git dependent tests. + uses: cylc/release-actions/configure-git@v1 + - name: Apt-Get Install if: startsWith(matrix.os, 'ubuntu') run: | diff --git a/metomi/rose/config_processors/fileinstall.py b/metomi/rose/config_processors/fileinstall.py index 24a839731a..e87045ce90 100644 --- a/metomi/rose/config_processors/fileinstall.py +++ b/metomi/rose/config_processors/fileinstall.py @@ -208,7 +208,7 @@ def _process(self, conf_tree, nodes, loc_dao, **kwargs): source.scheme = scheme break self.loc_handlers_manager.parse(source, conf_tree) - except ValueError: + except ValueError as exc: if source.is_optional: sources.pop(source.name) for name in source.used_by_names: @@ -220,6 +220,7 @@ def _process(self, conf_tree, nodes, loc_dao, **kwargs): raise ConfigProcessError( ["file:" + source.used_by_names[0], "source"], source.name, + exc ) prev_source = loc_dao.select(source.name) source.is_out_of_date = ( @@ -856,7 +857,7 @@ def parse(self, loc, conf_tree): # Scheme specified in the configuration. handler = self.get_handler(loc.scheme) if handler is None: - raise ValueError(loc.name) + raise ValueError(f"don't support scheme {loc.scheme}") else: # Scheme not specified in the configuration. scheme = urlparse(loc.name).scheme @@ -865,7 +866,7 @@ def parse(self, loc, conf_tree): if handler is None: handler = self.guess_handler(loc) if handler is None: - raise ValueError(loc.name) + raise ValueError(f"don't know how to process {loc.name}") else: handler = self.get_handler(self.DEFAULT_SCHEME) return handler.parse(loc, conf_tree) diff --git a/metomi/rose/loc_handlers/fs.py b/metomi/rose/loc_handlers/fs.py index 0c38c63090..0516144934 100644 --- a/metomi/rose/loc_handlers/fs.py +++ b/metomi/rose/loc_handlers/fs.py @@ -42,7 +42,7 @@ def parse(cls, loc, _): loc.scheme = "fs" name = os.path.expanduser(loc.name) if not os.path.exists(name): - raise ValueError(loc.name) + raise ValueError(f"path does not exist or not accessible: {name}") paths_and_checksums = get_checksum(name) for path, checksum, access_mode in paths_and_checksums: loc.add_path(path, checksum, access_mode) diff --git a/metomi/rose/loc_handlers/git.py b/metomi/rose/loc_handlers/git.py new file mode 100644 index 0000000000..a06841ea0f --- /dev/null +++ b/metomi/rose/loc_handlers/git.py @@ -0,0 +1,191 @@ +# Copyright (C) British Crown (Met Office) & Contributors. +# This file is part of Rose, a framework for meteorological suites. +# +# Rose is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# Rose is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Rose. If not, see . +# ----------------------------------------------------------------------------- +"""A handler of Git locations.""" + +import os +import re +import tempfile +from urllib.parse import urlparse + + +REC_COMMIT_HASH = re.compile(r"^[0-9a-f]+$") + + +class GitLocHandler: + """Handler of Git locations.""" + + GIT = "git" + SCHEMES = [GIT] + WEB_SCHEMES = ["https"] + URI_SEPARATOR = "::" + + def __init__(self, manager): + self.manager = manager + ret_code, versiontext, stderr = self.manager.popen.run( + "git", "version") + if ret_code: + self.git_version = None + else: + version_nums = [] + for num_string in versiontext.split()[-1].split("."): + try: + version_nums.append(int(num_string)) + except ValueError: + break + self.git_version = tuple(version_nums) + + def can_pull(self, loc): + if self.git_version is None: + return False + scheme = urlparse(loc.name).scheme + if scheme in self.SCHEMES: + return True + if self.URI_SEPARATOR not in loc.name: + return False + remote = self._parse_name(loc)[0] + return ( + scheme in self.WEB_SCHEMES + and not os.path.exists(loc.name) # same as svn... + and not self.manager.popen.run( + "git", "ls-remote", "--exit-code", remote)[0] + # https://superuser.com/questions/227509/git-ping-check-if-remote-repository-exists + ) + + def parse(self, loc, conf_tree): + """Set loc.real_name, loc.scheme, loc.loc_type. + + Within Git we have a lot of trouble figuring out remote + loc_type - a clone is required, unfortunately. + + Short commit hashes will not work since there is no remote + rev-parse functionality. Long commit hashes will work if the + uploadpack.allowAnySHA1InWant configuration is set on the + remote repo or server. + + Filtering requires uploadpack.allowFilter to be set true on + the remote repo or server. + + """ + loc.scheme = self.SCHEMES[0] + remote, path, ref = self._parse_name(loc) + with tempfile.TemporaryDirectory() as tmpdirname: + git_dir_opt = f"--git-dir={tmpdirname}/.git" + self.manager.popen.run_ok( + "git", git_dir_opt, "init" + ) + self.manager.popen.run_ok( + "git", git_dir_opt, "remote", "add", "origin", remote + ) + + # Make sure we configure for minimum fetching. + if self.git_version >= (2, 25, 0): + self.manager.popen.run_ok( + "git", git_dir_opt, "sparse-checkout", "set", path, + "--no-cone" + ) + else: + self.manager.popen.run_ok( + "git", git_dir_opt, "config", "extensions.partialClone", + "true" + ) + + # Extract the commit hash if we don't already have it. + commithash = self._get_commithash(remote, ref) + + # Fetch the ref/commit as efficiently as we can. + ret_code, _, stderr = self.manager.popen.run( + "git", git_dir_opt, "fetch", "--depth=1", + "--filter=blob:none", "origin", commithash + ) + if ret_code: + raise ValueError(f"source={loc.name}: {stderr}") + + # Determine the type of the path object. + ret_code, typetext, stderr = self.manager.popen.run( + "git", git_dir_opt, "cat-file", "-t", f"{commithash}:{path}" + ) # N.B. git versions >1.8 can use '-C' to set git dir. + if ret_code: + raise ValueError(f"source={loc.name}: {stderr}") + + if typetext.strip() == "tree": + loc.loc_type = loc.TYPE_TREE + else: + loc.loc_type = loc.TYPE_BLOB + loc.real_name = ( + f"remote:{remote} ref:{ref} commit:{commithash} path:{path}" + ) + loc.key = commithash + + async def pull(self, loc, conf_tree): + """Get loc to its cache. + + git sparse-checkout is not available below Git 2.25, and seems to omit + contents altogether if set to the root of the repo (as of 2.40.1). + + """ + if not loc.real_name: + self.parse(loc, conf_tree) + remote, path, ref = self._parse_name(loc) + with tempfile.TemporaryDirectory() as tmpdirname: + git_dir_opt = f"--git-dir={tmpdirname}/.git" + await self.manager.popen.run_ok_async( + "git", git_dir_opt, "init" + ) + await self.manager.popen.run_ok_async( + "git", git_dir_opt, "remote", "add", "origin", remote + ) + if self.git_version >= (2, 25, 0) and path != "./": + await self.manager.popen.run_ok_async( + "git", git_dir_opt, "sparse-checkout", "set", path, + "--no-cone" + ) + await self.manager.popen.run_ok_async( + "git", git_dir_opt, "fetch", "--depth=1", + "--filter=blob:none", "origin", loc.key + ) + else: + await self.manager.popen.run_ok_async( + "git", git_dir_opt, "fetch", "--depth=1", "origin", loc.key + ) + + await self.manager.popen.run_ok_async( + "git", git_dir_opt, f"--work-tree={tmpdirname}", "checkout", + loc.key + ) + name = tmpdirname + "/" + path + dest = loc.cache + if loc.loc_type == "tree": + dest += "/" + cmd = self.manager.popen.get_cmd("rsync", name, dest) + await self.manager.popen.run_ok_async(*cmd) + + def _parse_name(self, loc): + scheme, nonscheme = loc.name.split(":", 1) + return re.split(self.URI_SEPARATOR, nonscheme, maxsplit=3) + + def _get_commithash(self, remote, ref): + ret_code, info, _ = self.manager.popen.run( + "git", "ls-remote", "--exit-code", remote, ref) + if ret_code: + err = f"ls-remote: could not find ref '{ref}' in '{remote}'" + if REC_COMMIT_HASH.match(ref): + if len(ref) == 40: + # Likely a full commit hash. + return ref + err += ": you may be using an unsupported short commit hash" + raise ValueError(err) + return info.split()[0] diff --git a/metomi/rose/loc_handlers/namelist.py b/metomi/rose/loc_handlers/namelist.py index 17fca270ef..07166d14b8 100644 --- a/metomi/rose/loc_handlers/namelist.py +++ b/metomi/rose/loc_handlers/namelist.py @@ -61,7 +61,7 @@ def parse(self, loc, conf_tree): if section_value is None: sections.remove(section) if not sections: - raise ValueError(loc.name) + raise ValueError(f"could not locate {loc.name}") return sections async def pull(self, loc, conf_tree): diff --git a/metomi/rose/loc_handlers/rsync.py b/metomi/rose/loc_handlers/rsync.py index 2b7a9abb45..32d17a8add 100644 --- a/metomi/rose/loc_handlers/rsync.py +++ b/metomi/rose/loc_handlers/rsync.py @@ -67,7 +67,7 @@ def parse(self, loc, _): out = self.manager.popen(*cmd, stdin=stdin)[0] lines = out.splitlines() if not lines or lines[0] not in [loc.TYPE_BLOB, loc.TYPE_TREE]: - raise ValueError(loc.name) + raise ValueError(f"could not locate {path} on host {host}") loc.loc_type = lines.pop(0) if loc.loc_type == loc.TYPE_BLOB: line = lines.pop(0) diff --git a/sphinx/api/configuration/file-creation.rst b/sphinx/api/configuration/file-creation.rst index cab575ad46..fe565aaafd 100644 --- a/sphinx/api/configuration/file-creation.rst +++ b/sphinx/api/configuration/file-creation.rst @@ -25,6 +25,19 @@ root directory to install file targets with a relative path: :opt svn: The Subversion scheme. The location is a Subversion URL or an FCM location keyword. A URI with these schemes ``svn``, ``svn+ssh`` and ``fcm`` are automatically recognised. + :opt git: The Git scheme. The location is complex due to Git semantics. + It must have the scheme ``git`` and be of the form + ``git:REPOSITORY_URL::PATHSPEC::TREEISH``. ``REPOSITORY_URL`` should + be a Git repository URI which may itself have a scheme ``ssh``, + ``git``, ``https``, or be of the form ``HOST:PATH``, or ``PATH`` for + local repositories. ``PATHSPEC`` should be a path to a file or + directory that you want to extract. ``TREEISH`` should be a tag, + branch, or long commit hash to specify the commit at which you want + to extract. These should follow the same semantics as if you git + cloned ``REPOSITORY_URL``, git checkout'ed ``TREEISH``, and extracted + the path ``PATHSPEC`` within the clone. To extract from the root of + the repository use a ``PATHSPEC`` of ``./`` e.g. + ``git:git@github.com:metomi/rose::./::2.2.0``. :opt rsync: This scheme is useful for pulling a file or directory from a remote host using ``rsync`` via ``ssh``. A URI should have the form ``HOST:PATH``. diff --git a/sphinx/installation.rst b/sphinx/installation.rst index a787abd492..c586da76b4 100644 --- a/sphinx/installation.rst +++ b/sphinx/installation.rst @@ -67,6 +67,9 @@ non-Python dependencies are satisfied. + ``rose task-run`` + ``cylc install`` + Git is likewise required for installing files from Git repositories or + hosting services such as GitHub. Note Git is not automatically installed + by the metomi-rose conda. Configuring Rose ---------------- diff --git a/t/rose-app-run/05-file.t b/t/rose-app-run/05-file.t index f1fa0d51af..9ad0e993a1 100755 --- a/t/rose-app-run/05-file.t +++ b/t/rose-app-run/05-file.t @@ -44,7 +44,7 @@ Hello and good bye. __CONTENT__ OUT=$(cd config/file && cat hello1 hello2 hello3/text) #------------------------------------------------------------------------------- -tests 62 +tests 65 #------------------------------------------------------------------------------- # Normal mode with free format files. TEST_KEY=$TEST_KEY_BASE @@ -70,7 +70,18 @@ run_fail "$TEST_KEY" rose app-run --config=../config -q \ --define='[file:hello4]source=stuff:ing' file_cmp "$TEST_KEY.out" "$TEST_KEY.out" . +#------------------------------------------------------------------------------- +# Test "rose app-run", Git installation. +#------------------------------------------------------------------------------- +. $(dirname $0)/test_header + +mkdir $TEST_DIR/hellorepo +echo "Holly" >$TEST_DIR/hellorepo/tree.txt +echo "Timothy" >$TEST_DIR/hellorepo/grass.txt +mkdir $TEST_DIR/hellorepo/fruit +echo "Octavia" >$TEST_DIR/hellorepo/fruit/raspberry.txt +START_PWD=$PWD +cd $TEST_DIR/hellorepo +git init -q +git add . +git commit -m "Initial import" >/dev/null +MAIN_BRANCH=$(git branch --show-current) # main or master +COMMITHASH1=$(git rev-parse HEAD) +git tag -a v1.0 -m "Version 1 tagged" +git checkout -q -b branch1 +echo "Willow" >>$TEST_DIR/hellorepo/tree.txt +git commit -a -m "Add a tree" >/dev/null +git checkout -q $MAIN_BRANCH +echo "Clementine" >fruit/orange.txt +git add fruit +git commit -a -m "Add another fruit" >/dev/null +COMMITHASH2=$(git rev-parse HEAD) +git tag -a v2.0 -m "Version 2 tagged" +git config uploadpack.allowAnySHA1InWant true + +mkdir $TEST_DIR/git-http/ +git clone -q --bare $TEST_DIR/hellorepo $TEST_DIR/git-http/hellorepo.git +cd $TEST_DIR/git-http/hellorepo.git +git config uploadpack.allowFilter true +git config uploadpack.allowAnySHA1InWant true +touch git-daemon-export-ok +mkdir cgi-bin +BACKEND_LOCATION=$(locate --regex "/git-http-backend\$" | head -1) +if [[ -n "$BACKEND_LOCATION" ]]; then + ln -s $BACKEND_LOCATION cgi-bin/git +fi +GIT_WS_PORT="$((RANDOM + 10000))" +while port_is_busy "${GIT_WS_PORT}"; do + GIT_WS_PORT="$((RANDOM + 10000))" +done +python -c "import http.server; http.server.CGIHTTPRequestHandler.have_fork = False; http.server.test(HandlerClass=http.server.CGIHTTPRequestHandler, port=$GIT_WS_PORT)" >/dev/null 2>&1 & +GIT_WS_PID=${!} +sleep 10 +cd $START_PWD +#------------------------------------------------------------------------------- +tests 51 +#------------------------------------------------------------------------------- +remote_test_modes=("ssh" "http" "local") +remote_locations=("$HOSTNAME:$TEST_DIR/hellorepo/" "http://localhost:$GIT_WS_PORT/cgi-bin/git" "$TEST_DIR/hellorepo/") +for i in 0 1 2; do + remote_mode="${remote_test_modes[$i]}" + remote="${remote_locations[$i]}" + if [[ "$remote_mode" == "ssh" ]] && ! ssh -n -q -oBatchMode=yes $HOSTNAME true 1>'/dev/null' 2>/dev/null; then + skip 14 "cannot ssh to localhost $HOSTNAME" + echo "Skip $remote" >/dev/tty + continue + fi + if [[ "$remote_mode" == "http" ]] && ! curl --head --silent --fail $remote >/dev/null 2>&1; then + skip 14 "failed to launch http on localhost" + echo "Skip $remote" >/dev/tty + continue + fi + test_init <<__CONFIG__ +[command] +default=true + +[file:hello/fruit_main] +source=git:$remote::fruit::$MAIN_BRANCH + +[file:hello/fruit_tag1] +source=git:$remote::fruit::v1.0 + +[file:hello/fruit_tag2] +source=git:$remote::fruit::v2.0 + +[file:hello/fruit_commit1] +source=git:$remote::fruit::$COMMITHASH1 + +[file:hello/fruit_commit2] +source=git:$remote::fruit::$COMMITHASH2 + +[file:hello/tree_main.txt] +source=git:$remote::tree.txt::$MAIN_BRANCH + +[file:hello/tree_branch1.txt] +source=git:$remote::tree.txt::branch1 + +[file:hello/tree_tag1.txt] +source=git:$remote::tree.txt::v1.0 + +[file:hello/tree_tag2.txt] +source=git:$remote::tree.txt::v2.0 + +[file:hello/tree_commit1.txt] +source=git:$remote::tree.txt::$COMMITHASH1 +__CONFIG__ + TEST_KEY="$TEST_KEY_BASE-run-ok-$remote_mode" + test_setup + run_pass "$TEST_KEY" rose app-run --config=../config -q + find hello -type f | LANG=C sort >'find-hello.out' + file_cmp "$TEST_KEY.found" "find-hello.out" <<__CONTENT__ +hello/fruit_commit1/fruit/raspberry.txt +hello/fruit_commit2/fruit/orange.txt +hello/fruit_commit2/fruit/raspberry.txt +hello/fruit_main/fruit/orange.txt +hello/fruit_main/fruit/raspberry.txt +hello/fruit_tag1/fruit/raspberry.txt +hello/fruit_tag2/fruit/orange.txt +hello/fruit_tag2/fruit/raspberry.txt +hello/tree_branch1.txt +hello/tree_commit1.txt +hello/tree_main.txt +hello/tree_tag1.txt +hello/tree_tag2.txt +__CONTENT__ + file_cmp "$TEST_KEY.found_file0" "hello/fruit_commit1/fruit/raspberry.txt" <<__CONTENT__ +Octavia +__CONTENT__ + file_cmp "$TEST_KEY.found_file1" "hello/fruit_commit1/fruit/raspberry.txt" "hello/fruit_commit2/fruit/raspberry.txt" + file_cmp "$TEST_KEY.found_file2" "hello/fruit_commit1/fruit/raspberry.txt" "hello/fruit_main/fruit/raspberry.txt" + file_cmp "$TEST_KEY.found_file3" "hello/fruit_commit1/fruit/raspberry.txt" "hello/fruit_tag1/fruit/raspberry.txt" + file_cmp "$TEST_KEY.found_file4" "hello/fruit_commit2/fruit/orange.txt" <<__CONTENT__ +Clementine +__CONTENT__ + file_cmp "$TEST_KEY.found_file5" "hello/fruit_commit2/fruit/orange.txt" "hello/fruit_main/fruit/orange.txt" + file_cmp "$TEST_KEY.found_file6" "hello/fruit_commit2/fruit/orange.txt" "hello/fruit_tag2/fruit/orange.txt" + file_cmp "$TEST_KEY.found_file7" "hello/tree_commit1.txt" <<__CONTENT__ +Holly +__CONTENT__ + file_cmp "$TEST_KEY.found_file8" "hello/tree_commit1.txt" "hello/tree_main.txt" + file_cmp "$TEST_KEY.found_file9" "hello/tree_commit1.txt" "hello/tree_tag1.txt" + file_cmp "$TEST_KEY.found_file9" "hello/tree_commit1.txt" "hello/tree_tag2.txt" + file_cmp "$TEST_KEY.found_file10" "hello/tree_branch1.txt" <<__CONTENT__ +Holly +Willow +__CONTENT__ + test_teardown + repo_is_local_key="remote" +done + +kill "$GIT_WS_PID" +wait "$GIT_WS_PID" 2>/dev/null + +#------------------------------------------------------------------------------- +TEST_KEY="$TEST_KEY_BASE-null-change" +test_setup +rose app-run --config=../config -q || exit 1 +find hello -type f | LANG=C sort >'find-hello-before.out' +touch timeline # Nothing should be created after "timeline" +sleep 1 +run_pass "$TEST_KEY" rose app-run --config=../config -q +find hello -type f | LANG=C sort >'find-hello-after.out' +file_cmp "$TEST_KEY.find-hello" 'find-hello-before.out' 'find-hello-after.out' +find hello -type f -newer timeline | LANG=C sort >'find-hello-after-newer.out' +file_cmp "$TEST_KEY.find-hello-newer" 'find-hello-after-newer.out' 'find-hello.out' +file_cmp "$TEST_KEY.find" 'find-hello.out' <<__FILE__ +hello/fruit/orange.txt +hello/fruit/raspberry.txt +hello/grass.txt +hello/tree.txt +__FILE__ +test_teardown +#------------------------------------------------------------------------------- +TEST_KEY="$TEST_KEY_BASE-bad-ref" +test_setup +test_init <<__CONFIG__ +[command] +default=true + +[file:hello/fruit_main] +source=git:$TEST_DIR/hellorepo/::fruit::bad_ref +__CONFIG__ +run_fail "$TEST_KEY" rose app-run --config=../config +file_cmp "$TEST_KEY.err" "$TEST_KEY.err" <<__ERROR__ +[FAIL] file:hello/fruit_main=source=git:$TEST_DIR/hellorepo/::fruit::bad_ref: ls-remote: could not find ref 'bad_ref' in '$TEST_DIR/hellorepo/' +__ERROR__ +test_teardown +#------------------------------------------------------------------------------- +TEST_KEY="$TEST_KEY_BASE-bad-short-commit" +test_setup +test_init <<__CONFIG__ +[command] +default=true + +[file:hello/fruit_main] +source=git:$TEST_DIR/hellorepo/::fruit::${COMMITHASH1::7} +__CONFIG__ +run_fail "$TEST_KEY" rose app-run --config=../config +file_cmp "$TEST_KEY.err" "$TEST_KEY.err" <<__ERROR__ +[FAIL] file:hello/fruit_main=source=git:$TEST_DIR/hellorepo/::fruit::${COMMITHASH1::7}: ls-remote: could not find ref '${COMMITHASH1::7}' in '$TEST_DIR/hellorepo/': you may be using an unsupported short commit hash +__ERROR__ +test_teardown From 64a17e533585329d0c9c57d01eb0653b23f62063 Mon Sep 17 00:00:00 2001 From: Ben Fitzpatrick Date: Fri, 10 May 2024 08:17:20 +0100 Subject: [PATCH 02/12] Remove unnecessary/ineffective commands thanks to review --- metomi/rose/loc_handlers/git.py | 19 ++++------------- sphinx/api/configuration/file-creation.rst | 24 +++++++++++++++++++++- 2 files changed, 27 insertions(+), 16 deletions(-) diff --git a/metomi/rose/loc_handlers/git.py b/metomi/rose/loc_handlers/git.py index a06841ea0f..c37c807772 100644 --- a/metomi/rose/loc_handlers/git.py +++ b/metomi/rose/loc_handlers/git.py @@ -87,17 +87,9 @@ def parse(self, loc, conf_tree): self.manager.popen.run_ok( "git", git_dir_opt, "init" ) - self.manager.popen.run_ok( - "git", git_dir_opt, "remote", "add", "origin", remote - ) # Make sure we configure for minimum fetching. - if self.git_version >= (2, 25, 0): - self.manager.popen.run_ok( - "git", git_dir_opt, "sparse-checkout", "set", path, - "--no-cone" - ) - else: + if self.git_version < (2, 25, 0): self.manager.popen.run_ok( "git", git_dir_opt, "config", "extensions.partialClone", "true" @@ -109,7 +101,7 @@ def parse(self, loc, conf_tree): # Fetch the ref/commit as efficiently as we can. ret_code, _, stderr = self.manager.popen.run( "git", git_dir_opt, "fetch", "--depth=1", - "--filter=blob:none", "origin", commithash + "--filter=blob:none", remote, commithash ) if ret_code: raise ValueError(f"source={loc.name}: {stderr}") @@ -145,9 +137,6 @@ async def pull(self, loc, conf_tree): await self.manager.popen.run_ok_async( "git", git_dir_opt, "init" ) - await self.manager.popen.run_ok_async( - "git", git_dir_opt, "remote", "add", "origin", remote - ) if self.git_version >= (2, 25, 0) and path != "./": await self.manager.popen.run_ok_async( "git", git_dir_opt, "sparse-checkout", "set", path, @@ -155,11 +144,11 @@ async def pull(self, loc, conf_tree): ) await self.manager.popen.run_ok_async( "git", git_dir_opt, "fetch", "--depth=1", - "--filter=blob:none", "origin", loc.key + "--filter=blob:none", remote, loc.key ) else: await self.manager.popen.run_ok_async( - "git", git_dir_opt, "fetch", "--depth=1", "origin", loc.key + "git", git_dir_opt, "fetch", "--depth=1", remote, loc.key ) await self.manager.popen.run_ok_async( diff --git a/sphinx/api/configuration/file-creation.rst b/sphinx/api/configuration/file-creation.rst index fe565aaafd..47c19c3442 100644 --- a/sphinx/api/configuration/file-creation.rst +++ b/sphinx/api/configuration/file-creation.rst @@ -37,7 +37,29 @@ root directory to install file targets with a relative path: cloned ``REPOSITORY_URL``, git checkout'ed ``TREEISH``, and extracted the path ``PATHSPEC`` within the clone. To extract from the root of the repository use a ``PATHSPEC`` of ``./`` e.g. - ``git:git@github.com:metomi/rose::./::2.2.0``. + ``git:git@github.com:metomi/rose::./::2.2.0``. It may help to think + of the parts of the location as git:Where::What::When. Examples: + + .. code-block:: rose + + # Download the sphinx directory from the master branch of + # the github.com/metomi/rose repo. + [file:rose-docs] + source=git:git@github.com:metomi/rose::sphinx::master + + # Extract the whole contents of version 2.0.1 of the local + # repository at /home/user/some/path/to/my/git/repo. + [file:all_of_my_repo] + source=git:/home/user/some/path/to/my/git/repo::./::2.0.1 + + # Extract a single file from a particular commit of a repo + # on a machine that we have ssh access to. + [file:my_file] + source=git:machine01:/data/user/my_repo_name::etc/my_file::7261bff4d9a6c582ec759ef52c46dd794fe8794e + + You should set ``git config uploadpack.allowFilter true`` and + ``git config uploadpack.allowAnySHA1InWant true`` on repositories + if you are setting them up to pull from. :opt rsync: This scheme is useful for pulling a file or directory from a remote host using ``rsync`` via ``ssh``. A URI should have the form ``HOST:PATH``. From 5a16399086bcd7388515b265fbb90f3607050bc4 Mon Sep 17 00:00:00 2001 From: Ben Fitzpatrick Date: Tue, 4 Jun 2024 13:44:21 +0100 Subject: [PATCH 03/12] Minimal parsing, inferred loc type, fewer commands and less bandwidth --- metomi/rose/loc_handlers/git.py | 66 +++++++---------- sphinx/api/configuration/file-creation.rst | 9 +-- t/rose-app-run/28-git.t | 82 +++++++++++++++------- 3 files changed, 89 insertions(+), 68 deletions(-) diff --git a/metomi/rose/loc_handlers/git.py b/metomi/rose/loc_handlers/git.py index c37c807772..b287ef1a44 100644 --- a/metomi/rose/loc_handlers/git.py +++ b/metomi/rose/loc_handlers/git.py @@ -69,51 +69,20 @@ def parse(self, loc, conf_tree): """Set loc.real_name, loc.scheme, loc.loc_type. Within Git we have a lot of trouble figuring out remote - loc_type - a clone is required, unfortunately. - - Short commit hashes will not work since there is no remote - rev-parse functionality. Long commit hashes will work if the - uploadpack.allowAnySHA1InWant configuration is set on the - remote repo or server. - - Filtering requires uploadpack.allowFilter to be set true on - the remote repo or server. + loc_type - a clone is required, unfortunately. There is a + tradeoff between extra Git commands and bandwidth vs + parse failure behaviour. We have decided to short cut the + loc_type calculation to save commands and bandwidth, + catching failures later. """ loc.scheme = self.SCHEMES[0] remote, path, ref = self._parse_name(loc) - with tempfile.TemporaryDirectory() as tmpdirname: - git_dir_opt = f"--git-dir={tmpdirname}/.git" - self.manager.popen.run_ok( - "git", git_dir_opt, "init" - ) - # Make sure we configure for minimum fetching. - if self.git_version < (2, 25, 0): - self.manager.popen.run_ok( - "git", git_dir_opt, "config", "extensions.partialClone", - "true" - ) - - # Extract the commit hash if we don't already have it. - commithash = self._get_commithash(remote, ref) + # Extract the commit hash if we don't already have it. + commithash = self._get_commithash(remote, ref) - # Fetch the ref/commit as efficiently as we can. - ret_code, _, stderr = self.manager.popen.run( - "git", git_dir_opt, "fetch", "--depth=1", - "--filter=blob:none", remote, commithash - ) - if ret_code: - raise ValueError(f"source={loc.name}: {stderr}") - - # Determine the type of the path object. - ret_code, typetext, stderr = self.manager.popen.run( - "git", git_dir_opt, "cat-file", "-t", f"{commithash}:{path}" - ) # N.B. git versions >1.8 can use '-C' to set git dir. - if ret_code: - raise ValueError(f"source={loc.name}: {stderr}") - - if typetext.strip() == "tree": + if path.endswith("/"): # This is a short cut, checked later. loc.loc_type = loc.TYPE_TREE else: loc.loc_type = loc.TYPE_BLOB @@ -128,6 +97,9 @@ async def pull(self, loc, conf_tree): git sparse-checkout is not available below Git 2.25, and seems to omit contents altogether if set to the root of the repo (as of 2.40.1). + Filtering requires uploadpack.allowFilter to be set true on + the remote repo or server. + """ if not loc.real_name: self.parse(loc, conf_tree) @@ -156,6 +128,14 @@ async def pull(self, loc, conf_tree): loc.key ) name = tmpdirname + "/" + path + + # Check that we have inferred the right type from the path name. + real_loc_type = loc.TYPE_TREE if os.path.isdir(name) else loc.TYPE_BLOB + if real_loc_type != loc.loc_type: + raise ValueError( + f"Expected path '{path}' to be type '{loc.loc_type}', " + + f"but it was '{real_loc_type}'. Check trailing slash." + ) dest = loc.cache if loc.loc_type == "tree": dest += "/" @@ -167,6 +147,14 @@ def _parse_name(self, loc): return re.split(self.URI_SEPARATOR, nonscheme, maxsplit=3) def _get_commithash(self, remote, ref): + """Get the commit hash given a branch, tag, or commit hash. + + Short commit hashes will not resolve since there is no remote + rev-parse functionality. Long commit hashes will work if the + uploadpack.allowAnySHA1InWant configuration is set on the + remote repo or server. + + """ ret_code, info, _ = self.manager.popen.run( "git", "ls-remote", "--exit-code", remote, ref) if ret_code: diff --git a/sphinx/api/configuration/file-creation.rst b/sphinx/api/configuration/file-creation.rst index 47c19c3442..e40d0431df 100644 --- a/sphinx/api/configuration/file-creation.rst +++ b/sphinx/api/configuration/file-creation.rst @@ -35,17 +35,18 @@ root directory to install file targets with a relative path: branch, or long commit hash to specify the commit at which you want to extract. These should follow the same semantics as if you git cloned ``REPOSITORY_URL``, git checkout'ed ``TREEISH``, and extracted - the path ``PATHSPEC`` within the clone. To extract from the root of - the repository use a ``PATHSPEC`` of ``./`` e.g. + the path ``PATHSPEC`` within the clone. ``PATHSPEC`` must end with a + trailing slash (``/``) if it is a directory. To extract from the root + of the repository use a ``PATHSPEC`` of ``./`` e.g. ``git:git@github.com:metomi/rose::./::2.2.0``. It may help to think of the parts of the location as git:Where::What::When. Examples: .. code-block:: rose # Download the sphinx directory from the master branch of - # the github.com/metomi/rose repo. + # the github.com/metomi/rose repo. Note trailing slash. [file:rose-docs] - source=git:git@github.com:metomi/rose::sphinx::master + source=git:git@github.com:metomi/rose::sphinx/::master # Extract the whole contents of version 2.0.1 of the local # repository at /home/user/some/path/to/my/git/repo. diff --git a/t/rose-app-run/28-git.t b/t/rose-app-run/28-git.t index b3cc0ddadf..6cbc942dfa 100644 --- a/t/rose-app-run/28-git.t +++ b/t/rose-app-run/28-git.t @@ -65,7 +65,7 @@ GIT_WS_PID=${!} sleep 10 cd $START_PWD #------------------------------------------------------------------------------- -tests 51 +tests 55 #------------------------------------------------------------------------------- remote_test_modes=("ssh" "http" "local") remote_locations=("$HOSTNAME:$TEST_DIR/hellorepo/" "http://localhost:$GIT_WS_PORT/cgi-bin/git" "$TEST_DIR/hellorepo/") @@ -87,19 +87,19 @@ for i in 0 1 2; do default=true [file:hello/fruit_main] -source=git:$remote::fruit::$MAIN_BRANCH +source=git:$remote::fruit/::$MAIN_BRANCH [file:hello/fruit_tag1] -source=git:$remote::fruit::v1.0 +source=git:$remote::fruit/::v1.0 [file:hello/fruit_tag2] -source=git:$remote::fruit::v2.0 +source=git:$remote::fruit/::v2.0 [file:hello/fruit_commit1] -source=git:$remote::fruit::$COMMITHASH1 +source=git:$remote::fruit/::$COMMITHASH1 [file:hello/fruit_commit2] -source=git:$remote::fruit::$COMMITHASH2 +source=git:$remote::fruit/::$COMMITHASH2 [file:hello/tree_main.txt] source=git:$remote::tree.txt::$MAIN_BRANCH @@ -121,31 +121,31 @@ __CONFIG__ run_pass "$TEST_KEY" rose app-run --config=../config -q find hello -type f | LANG=C sort >'find-hello.out' file_cmp "$TEST_KEY.found" "find-hello.out" <<__CONTENT__ -hello/fruit_commit1/fruit/raspberry.txt -hello/fruit_commit2/fruit/orange.txt -hello/fruit_commit2/fruit/raspberry.txt -hello/fruit_main/fruit/orange.txt -hello/fruit_main/fruit/raspberry.txt -hello/fruit_tag1/fruit/raspberry.txt -hello/fruit_tag2/fruit/orange.txt -hello/fruit_tag2/fruit/raspberry.txt +hello/fruit_commit1/raspberry.txt +hello/fruit_commit2/orange.txt +hello/fruit_commit2/raspberry.txt +hello/fruit_main/orange.txt +hello/fruit_main/raspberry.txt +hello/fruit_tag1/raspberry.txt +hello/fruit_tag2/orange.txt +hello/fruit_tag2/raspberry.txt hello/tree_branch1.txt hello/tree_commit1.txt hello/tree_main.txt hello/tree_tag1.txt hello/tree_tag2.txt __CONTENT__ - file_cmp "$TEST_KEY.found_file0" "hello/fruit_commit1/fruit/raspberry.txt" <<__CONTENT__ + file_cmp "$TEST_KEY.found_file0" "hello/fruit_commit1/raspberry.txt" <<__CONTENT__ Octavia __CONTENT__ - file_cmp "$TEST_KEY.found_file1" "hello/fruit_commit1/fruit/raspberry.txt" "hello/fruit_commit2/fruit/raspberry.txt" - file_cmp "$TEST_KEY.found_file2" "hello/fruit_commit1/fruit/raspberry.txt" "hello/fruit_main/fruit/raspberry.txt" - file_cmp "$TEST_KEY.found_file3" "hello/fruit_commit1/fruit/raspberry.txt" "hello/fruit_tag1/fruit/raspberry.txt" - file_cmp "$TEST_KEY.found_file4" "hello/fruit_commit2/fruit/orange.txt" <<__CONTENT__ + file_cmp "$TEST_KEY.found_file1" "hello/fruit_commit1/raspberry.txt" "hello/fruit_commit2/raspberry.txt" + file_cmp "$TEST_KEY.found_file2" "hello/fruit_commit1/raspberry.txt" "hello/fruit_main/raspberry.txt" + file_cmp "$TEST_KEY.found_file3" "hello/fruit_commit1/raspberry.txt" "hello/fruit_tag1/raspberry.txt" + file_cmp "$TEST_KEY.found_file4" "hello/fruit_commit2/orange.txt" <<__CONTENT__ Clementine __CONTENT__ - file_cmp "$TEST_KEY.found_file5" "hello/fruit_commit2/fruit/orange.txt" "hello/fruit_main/fruit/orange.txt" - file_cmp "$TEST_KEY.found_file6" "hello/fruit_commit2/fruit/orange.txt" "hello/fruit_tag2/fruit/orange.txt" + file_cmp "$TEST_KEY.found_file5" "hello/fruit_commit2/orange.txt" "hello/fruit_main/orange.txt" + file_cmp "$TEST_KEY.found_file6" "hello/fruit_commit2/orange.txt" "hello/fruit_tag2/orange.txt" file_cmp "$TEST_KEY.found_file7" "hello/tree_commit1.txt" <<__CONTENT__ Holly __CONTENT__ @@ -203,11 +203,11 @@ test_init <<__CONFIG__ default=true [file:hello/fruit_main] -source=git:$TEST_DIR/hellorepo/::fruit::bad_ref +source=git:$TEST_DIR/hellorepo/::fruit/::bad_ref __CONFIG__ run_fail "$TEST_KEY" rose app-run --config=../config file_cmp "$TEST_KEY.err" "$TEST_KEY.err" <<__ERROR__ -[FAIL] file:hello/fruit_main=source=git:$TEST_DIR/hellorepo/::fruit::bad_ref: ls-remote: could not find ref 'bad_ref' in '$TEST_DIR/hellorepo/' +[FAIL] file:hello/fruit_main=source=git:$TEST_DIR/hellorepo/::fruit/::bad_ref: ls-remote: could not find ref 'bad_ref' in '$TEST_DIR/hellorepo/' __ERROR__ test_teardown #------------------------------------------------------------------------------- @@ -218,10 +218,42 @@ test_init <<__CONFIG__ default=true [file:hello/fruit_main] -source=git:$TEST_DIR/hellorepo/::fruit::${COMMITHASH1::7} +source=git:$TEST_DIR/hellorepo/::fruit/::${COMMITHASH1::7} __CONFIG__ run_fail "$TEST_KEY" rose app-run --config=../config file_cmp "$TEST_KEY.err" "$TEST_KEY.err" <<__ERROR__ -[FAIL] file:hello/fruit_main=source=git:$TEST_DIR/hellorepo/::fruit::${COMMITHASH1::7}: ls-remote: could not find ref '${COMMITHASH1::7}' in '$TEST_DIR/hellorepo/': you may be using an unsupported short commit hash +[FAIL] file:hello/fruit_main=source=git:$TEST_DIR/hellorepo/::fruit/::${COMMITHASH1::7}: ls-remote: could not find ref '${COMMITHASH1::7}' in '$TEST_DIR/hellorepo/': you may be using an unsupported short commit hash +__ERROR__ +test_teardown +#------------------------------------------------------------------------------- +TEST_KEY="$TEST_KEY_BASE-bad-path-blob-should-be-tree" +test_setup +test_init <<__CONFIG__ +[command] +default=true + +[file:hello/fruit_main] +source=git:$TEST_DIR/hellorepo::fruit::$MAIN_BRANCH +__CONFIG__ +run_fail "$TEST_KEY" rose app-run --config=../config +file_cmp "$TEST_KEY.err" "$TEST_KEY.err" <<__ERROR__ +[FAIL] Expected path 'fruit' to be type 'blob', but it was 'tree'. Check trailing slash. +[FAIL] source: remote:$TEST_DIR/hellorepo ref:$MAIN_BRANCH commit:$COMMITHASH2 path:fruit (git:$TEST_DIR/hellorepo::fruit::$MAIN_BRANCH) +__ERROR__ +test_teardown +#------------------------------------------------------------------------------- +TEST_KEY="$TEST_KEY_BASE-bad-path-tree-should-be-blob" +test_setup +test_init <<__CONFIG__ +[command] +default=true + +[file:hello/fruit_main/txt] +source=git:$TEST_DIR/hellorepo::fruit/orange.txt/::$MAIN_BRANCH +__CONFIG__ +run_fail "$TEST_KEY" rose app-run --config=../config +file_cmp "$TEST_KEY.err" "$TEST_KEY.err" <<__ERROR__ +[FAIL] Expected path 'fruit/orange.txt/' to be type 'tree', but it was 'blob'. Check trailing slash. +[FAIL] source: remote:$TEST_DIR/hellorepo ref:$MAIN_BRANCH commit:$COMMITHASH2 path:fruit/orange.txt/ (git:$TEST_DIR/hellorepo::fruit/orange.txt/::$MAIN_BRANCH) __ERROR__ test_teardown From e54d5040f4f3768f78fc90a857c1277412c6d374 Mon Sep 17 00:00:00 2001 From: Ben Fitzpatrick Date: Wed, 5 Jun 2024 11:38:47 +0100 Subject: [PATCH 04/12] Update metomi/rose/loc_handlers/git.py Co-authored-by: Oliver Sanders --- metomi/rose/loc_handlers/git.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/metomi/rose/loc_handlers/git.py b/metomi/rose/loc_handlers/git.py index b287ef1a44..190de79084 100644 --- a/metomi/rose/loc_handlers/git.py +++ b/metomi/rose/loc_handlers/git.py @@ -130,7 +130,9 @@ async def pull(self, loc, conf_tree): name = tmpdirname + "/" + path # Check that we have inferred the right type from the path name. - real_loc_type = loc.TYPE_TREE if os.path.isdir(name) else loc.TYPE_BLOB + real_loc_type = ( + loc.TYPE_TREE if os.path.isdir(name) else loc.TYPE_BLOB + ) if real_loc_type != loc.loc_type: raise ValueError( f"Expected path '{path}' to be type '{loc.loc_type}', " From b4f653fc4dbdaa70399bb520cbc695b716b43d2f Mon Sep 17 00:00:00 2001 From: Ben Fitzpatrick Date: Wed, 5 Jun 2024 11:40:25 +0100 Subject: [PATCH 05/12] More comments --- metomi/rose/loc_handlers/git.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/metomi/rose/loc_handlers/git.py b/metomi/rose/loc_handlers/git.py index 190de79084..b79335c8d5 100644 --- a/metomi/rose/loc_handlers/git.py +++ b/metomi/rose/loc_handlers/git.py @@ -35,11 +35,14 @@ class GitLocHandler: def __init__(self, manager): self.manager = manager + # Determine (just once) what git version we have, if any. ret_code, versiontext, stderr = self.manager.popen.run( "git", "version") if ret_code: + # Git not installed. self.git_version = None else: + # Git is installed, get the version. version_nums = [] for num_string in versiontext.split()[-1].split("."): try: @@ -49,6 +52,7 @@ def __init__(self, manager): self.git_version = tuple(version_nums) def can_pull(self, loc): + """Determine if this is a suitable handler for loc.""" if self.git_version is None: return False scheme = urlparse(loc.name).scheme @@ -89,7 +93,7 @@ def parse(self, loc, conf_tree): loc.real_name = ( f"remote:{remote} ref:{ref} commit:{commithash} path:{path}" ) - loc.key = commithash + loc.key = commithash # We'll notice branch/tag updates. async def pull(self, loc, conf_tree): """Get loc to its cache. @@ -110,6 +114,7 @@ async def pull(self, loc, conf_tree): "git", git_dir_opt, "init" ) if self.git_version >= (2, 25, 0) and path != "./": + # sparse-checkout available and suitable for this case. await self.manager.popen.run_ok_async( "git", git_dir_opt, "sparse-checkout", "set", path, "--no-cone" @@ -119,10 +124,12 @@ async def pull(self, loc, conf_tree): "--filter=blob:none", remote, loc.key ) else: + # Fallback. await self.manager.popen.run_ok_async( "git", git_dir_opt, "fetch", "--depth=1", remote, loc.key ) + # Checkout to temporary location, then extract only 'path' later. await self.manager.popen.run_ok_async( "git", git_dir_opt, f"--work-tree={tmpdirname}", "checkout", loc.key @@ -138,6 +145,8 @@ async def pull(self, loc, conf_tree): f"Expected path '{path}' to be type '{loc.loc_type}', " + f"but it was '{real_loc_type}'. Check trailing slash." ) + + # Extract only 'path' to cache. dest = loc.cache if loc.loc_type == "tree": dest += "/" From 0390b4901c489477b8a1b713910f623027d3ebbf Mon Sep 17 00:00:00 2001 From: Ben Fitzpatrick Date: Tue, 11 Jun 2024 14:18:46 +0100 Subject: [PATCH 06/12] Update sphinx/api/configuration/file-creation.rst Co-authored-by: Oliver Sanders --- sphinx/api/configuration/file-creation.rst | 33 ++++++++++++++-------- 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/sphinx/api/configuration/file-creation.rst b/sphinx/api/configuration/file-creation.rst index e40d0431df..1e32d03cc0 100644 --- a/sphinx/api/configuration/file-creation.rst +++ b/sphinx/api/configuration/file-creation.rst @@ -27,19 +27,28 @@ root directory to install file targets with a relative path: ``svn+ssh`` and ``fcm`` are automatically recognised. :opt git: The Git scheme. The location is complex due to Git semantics. It must have the scheme ``git`` and be of the form - ``git:REPOSITORY_URL::PATHSPEC::TREEISH``. ``REPOSITORY_URL`` should - be a Git repository URI which may itself have a scheme ``ssh``, - ``git``, ``https``, or be of the form ``HOST:PATH``, or ``PATH`` for - local repositories. ``PATHSPEC`` should be a path to a file or - directory that you want to extract. ``TREEISH`` should be a tag, - branch, or long commit hash to specify the commit at which you want - to extract. These should follow the same semantics as if you git + ``git:REPOSITORY_URL::PATHSPEC::TREEISH``: + + * ``REPOSITORY_URL`` should + be a Git repository URI which may itself have a scheme ``ssh``, + ``git``, ``https``, or be of the form ``HOST:PATH``, or ``PATH`` for + local repositories. + * ``PATHSPEC`` should be a path to a file or + directory that you want to extract. + The ``PATHSPEC`` must end with a + trailing slash (``/``) if it is a directory. To extract from the root + of the repository use a ``PATHSPEC`` of ``./`` e.g. + ``git:git@github.com:metomi/rose::./::2.2.0``. + * ``TREEISH`` should be a tag, + branch, or long commit hash to specify the commit at which you want + to extract. + + These should follow the same semantics as if you git cloned ``REPOSITORY_URL``, git checkout'ed ``TREEISH``, and extracted - the path ``PATHSPEC`` within the clone. ``PATHSPEC`` must end with a - trailing slash (``/``) if it is a directory. To extract from the root - of the repository use a ``PATHSPEC`` of ``./`` e.g. - ``git:git@github.com:metomi/rose::./::2.2.0``. It may help to think - of the parts of the location as git:Where::What::When. Examples: + the path ``PATHSPEC`` within the clone. It may help to think + of the parts of the location as ``git:Where::What::When``. + + ..rubric:: Examples: .. code-block:: rose From 5bc2f97c70732879ba38ef27625ef15b3cf7a198 Mon Sep 17 00:00:00 2001 From: Ben Fitzpatrick Date: Thu, 13 Jun 2024 08:48:38 +0100 Subject: [PATCH 07/12] Update metomi/rose/loc_handlers/git.py Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com> --- metomi/rose/loc_handlers/git.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/metomi/rose/loc_handlers/git.py b/metomi/rose/loc_handlers/git.py index b79335c8d5..32253fbbd0 100644 --- a/metomi/rose/loc_handlers/git.py +++ b/metomi/rose/loc_handlers/git.py @@ -155,7 +155,7 @@ async def pull(self, loc, conf_tree): def _parse_name(self, loc): scheme, nonscheme = loc.name.split(":", 1) - return re.split(self.URI_SEPARATOR, nonscheme, maxsplit=3) + return nonscheme.split(self.URI_SEPARATOR, maxsplit=3) def _get_commithash(self, remote, ref): """Get the commit hash given a branch, tag, or commit hash. From fab495cc8597d722ca550a3f76627166c74d3705 Mon Sep 17 00:00:00 2001 From: Ben Fitzpatrick Date: Thu, 13 Jun 2024 09:23:51 +0100 Subject: [PATCH 08/12] #1419: address @MetRonnie's feedback --- metomi/rose/loc_handlers/git.py | 9 ++++----- sphinx/api/configuration/file-creation.rst | 4 ++-- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/metomi/rose/loc_handlers/git.py b/metomi/rose/loc_handlers/git.py index 32253fbbd0..c206c763e1 100644 --- a/metomi/rose/loc_handlers/git.py +++ b/metomi/rose/loc_handlers/git.py @@ -161,9 +161,7 @@ def _get_commithash(self, remote, ref): """Get the commit hash given a branch, tag, or commit hash. Short commit hashes will not resolve since there is no remote - rev-parse functionality. Long commit hashes will work if the - uploadpack.allowAnySHA1InWant configuration is set on the - remote repo or server. + rev-parse functionality. """ ret_code, info, _ = self.manager.popen.run( @@ -171,8 +169,9 @@ def _get_commithash(self, remote, ref): if ret_code: err = f"ls-remote: could not find ref '{ref}' in '{remote}'" if REC_COMMIT_HASH.match(ref): - if len(ref) == 40: - # Likely a full commit hash. + if len(ref) in [40, 64]: # SHA1, SHA256 hashes. + # Likely a full commit hash, but the server + # uploadpack.allowAnySHA1InWant configuration is not set. return ref err += ": you may be using an unsupported short commit hash" raise ValueError(err) diff --git a/sphinx/api/configuration/file-creation.rst b/sphinx/api/configuration/file-creation.rst index 1e32d03cc0..fc7ecf7768 100644 --- a/sphinx/api/configuration/file-creation.rst +++ b/sphinx/api/configuration/file-creation.rst @@ -68,8 +68,8 @@ root directory to install file targets with a relative path: source=git:machine01:/data/user/my_repo_name::etc/my_file::7261bff4d9a6c582ec759ef52c46dd794fe8794e You should set ``git config uploadpack.allowFilter true`` and - ``git config uploadpack.allowAnySHA1InWant true`` on repositories - if you are setting them up to pull from. + optionally ``git config uploadpack.allowAnySHA1InWant true`` on + repositories if you are setting them up to pull from. :opt rsync: This scheme is useful for pulling a file or directory from a remote host using ``rsync`` via ``ssh``. A URI should have the form ``HOST:PATH``. From 0c5af7f787100fdb6e91824bac223016441d2c9c Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Mon, 17 Jun 2024 15:16:51 +0100 Subject: [PATCH 09/12] git: differentiate between no such branch and no such repo --- metomi/rose/loc_handlers/git.py | 3 +++ t/rose-app-run/28-git.t | 17 ++++++++++++++++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/metomi/rose/loc_handlers/git.py b/metomi/rose/loc_handlers/git.py index c206c763e1..8102b592f4 100644 --- a/metomi/rose/loc_handlers/git.py +++ b/metomi/rose/loc_handlers/git.py @@ -166,6 +166,9 @@ def _get_commithash(self, remote, ref): """ ret_code, info, _ = self.manager.popen.run( "git", "ls-remote", "--exit-code", remote, ref) + if ret_code == 128: + # repo not found + raise ValueError(f"ls-remote: could not locate '{remote}'") if ret_code: err = f"ls-remote: could not find ref '{ref}' in '{remote}'" if REC_COMMIT_HASH.match(ref): diff --git a/t/rose-app-run/28-git.t b/t/rose-app-run/28-git.t index 6cbc942dfa..1b4eaaa902 100644 --- a/t/rose-app-run/28-git.t +++ b/t/rose-app-run/28-git.t @@ -65,7 +65,7 @@ GIT_WS_PID=${!} sleep 10 cd $START_PWD #------------------------------------------------------------------------------- -tests 55 +tests 57 #------------------------------------------------------------------------------- remote_test_modes=("ssh" "http" "local") remote_locations=("$HOSTNAME:$TEST_DIR/hellorepo/" "http://localhost:$GIT_WS_PORT/cgi-bin/git" "$TEST_DIR/hellorepo/") @@ -196,6 +196,21 @@ hello/tree.txt __FILE__ test_teardown #------------------------------------------------------------------------------- +TEST_KEY="$TEST_KEY_BASE-bad-repo" +test_setup +test_init <<__CONFIG__ +[command] +default=true + +[file:hello/fruit_main] +source=git:$TEST_DIR/zz9+zα/::earth/::v1 +__CONFIG__ +run_fail "$TEST_KEY" rose app-run --config=../config +file_cmp "$TEST_KEY.err" "$TEST_KEY.err" <<__ERROR__ +[FAIL] file:hello/fruit_main=source=git:$TEST_DIR/zz9+zα/::earth/::v1: ls-remote: could not locate '$TEST_DIR/zz9+zα/' +__ERROR__ +test_teardown +#------------------------------------------------------------------------------- TEST_KEY="$TEST_KEY_BASE-bad-ref" test_setup test_init <<__CONFIG__ From 83623bc599843f23082b96b8af41284e3e04fa78 Mon Sep 17 00:00:00 2001 From: Oliver Sanders Date: Mon, 17 Jun 2024 16:29:27 +0100 Subject: [PATCH 10/12] Update metomi/rose/loc_handlers/git.py --- metomi/rose/loc_handlers/git.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/metomi/rose/loc_handlers/git.py b/metomi/rose/loc_handlers/git.py index 8102b592f4..c732ac540f 100644 --- a/metomi/rose/loc_handlers/git.py +++ b/metomi/rose/loc_handlers/git.py @@ -166,7 +166,7 @@ def _get_commithash(self, remote, ref): """ ret_code, info, _ = self.manager.popen.run( "git", "ls-remote", "--exit-code", remote, ref) - if ret_code == 128: + if ret_code != 2: # repo not found raise ValueError(f"ls-remote: could not locate '{remote}'") if ret_code: From 4851edae12242dcdf0e49c2ba274f1487f99042a Mon Sep 17 00:00:00 2001 From: Ben Fitzpatrick Date: Tue, 18 Jun 2024 10:46:13 +0100 Subject: [PATCH 11/12] Fix mistake, tests pass --- metomi/rose/loc_handlers/git.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/metomi/rose/loc_handlers/git.py b/metomi/rose/loc_handlers/git.py index c732ac540f..e6f6b932a3 100644 --- a/metomi/rose/loc_handlers/git.py +++ b/metomi/rose/loc_handlers/git.py @@ -166,7 +166,7 @@ def _get_commithash(self, remote, ref): """ ret_code, info, _ = self.manager.popen.run( "git", "ls-remote", "--exit-code", remote, ref) - if ret_code != 2: + if ret_code and ret_code != 2: # repo not found raise ValueError(f"ls-remote: could not locate '{remote}'") if ret_code: From 460bbf7db260cda565a5177b691da183e79c8c9e Mon Sep 17 00:00:00 2001 From: Ben Fitzpatrick Date: Tue, 18 Jun 2024 11:21:12 +0100 Subject: [PATCH 12/12] Fix test when all previous ones skipped --- t/rose-app-run/28-git.t | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/t/rose-app-run/28-git.t b/t/rose-app-run/28-git.t index 1b4eaaa902..2db5a1f03c 100644 --- a/t/rose-app-run/28-git.t +++ b/t/rose-app-run/28-git.t @@ -166,6 +166,40 @@ wait "$GIT_WS_PID" 2>/dev/null #------------------------------------------------------------------------------- TEST_KEY="$TEST_KEY_BASE-null-change" test_setup +test_init <<__CONFIG__ +[command] +default=true + +[file:hello/fruit_main] +source=git:$remote::fruit/::$MAIN_BRANCH + +[file:hello/fruit_tag1] +source=git:$remote::fruit/::v1.0 + +[file:hello/fruit_tag2] +source=git:$remote::fruit/::v2.0 + +[file:hello/fruit_commit1] +source=git:$remote::fruit/::$COMMITHASH1 + +[file:hello/fruit_commit2] +source=git:$remote::fruit/::$COMMITHASH2 + +[file:hello/tree_main.txt] +source=git:$remote::tree.txt::$MAIN_BRANCH + +[file:hello/tree_branch1.txt] +source=git:$remote::tree.txt::branch1 + +[file:hello/tree_tag1.txt] +source=git:$remote::tree.txt::v1.0 + +[file:hello/tree_tag2.txt] +source=git:$remote::tree.txt::v2.0 + +[file:hello/tree_commit1.txt] +source=git:$remote::tree.txt::$COMMITHASH1 +__CONFIG__ rose app-run --config=../config -q || exit 1 find hello -type f | LANG=C sort >'find-hello-before.out' touch timeline # Nothing should be created after "timeline"