Enable commit-specific git fetch for compatibility with external forks. - #83
leander-dsouza wants to merge 3 commits into
Conversation
85222a3 to
e4d26d4
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #83 +/- ##
=======================================
Coverage 26.76% 26.76%
=======================================
Files 31 31
Lines 2298 2298
Branches 405 408 +3
=======================================
Hits 615 615
Misses 1623 1623
Partials 60 60 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull Request Overview
This pull request adds support for fetching specific commit hashes in non-shallow Git import mode, and improves test skipping for VCS tools that may not be installed.
- Refactored version type determination to work for both shallow and non-shallow modes
- Added explicit fetch of commit hashes for non-shallow repository imports
- Added test skipping when SVN or Mercurial tools are not available
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| vcs2l/clients/git.py | Refactored to determine version type earlier and fetch specific commit hashes in non-shallow mode for both existing and new repositories |
| test/test_commands.py | Added utility variables and skip decorator for tests requiring SVN and Mercurial |
| test/reimport_force.txt | Updated expected output to include fetch messages for commit hash imports |
| test/import.txt | Updated expected output to include fetch messages for commit hash imports |
There was a problem hiding this comment.
The checks added here are part of the fixture setup in __init__.py. What's the motivation to duplicate them here?
There was a problem hiding this comment.
They were initially duplicated as Subversion, and Mercurial ignores were flaky in Windows 2025 in previous workflow runs.
However, they currently work as expected, so I have dropped the additional commit.
e4d26d4 to
9d6d6bc
Compare
9d6d6bc to
1fef30f
Compare
KmoM88
left a comment
There was a problem hiding this comment.
Overall LGTM.
I have left a suggestion regarding the execution workflow.
Also as a side note: What do you think about adding a dedicated test case that requests a commit from an unmerged, orphaned branch (simulating a real external fork) to prevent any future regressions?
Nice work, waiting to hear your thoughts.
| output = result_clone['output'] | ||
|
|
||
| # For non-shallow clones with commit hashes, fetch the specific commit | ||
| if not command.shallow and version_type == 'hash': |
There was a problem hiding this comment.
For a non-shallow commit-specific clone this is doing a full git clone and then does a git fetch for the target hash. For large repositories, this fetches a lot of unnecessary data.
What would you think about bypassing git clone entirely and using the git init + git fetch <hash> path for all precise tags and hashes (both shallow and non-shallow)?
Adjusting the conditional to redirect any hash or tag to the else block and toggling the --depth 1 flag on the fetch command dynamically if command.shallow is True.
This could save significant bandwidth for repositories with large default branches.
What do you think?
There was a problem hiding this comment.
Yes, I agree.
Now, hash/tag imports go through git init + git fetch <ref> for both shallow and non-shallow clones, with the --depth 1 added for shallow ones.
One thing I ran into in the hash case is that git fetch origin <hash> does not pull any tags, so git describe --tags breaks. Therefore, I have added the --tags to the fetch command to preserve it, and now subsequent expect .txt files have been updated to match it.
Signed-off-by: Leander Stephen D'Souza <leanderdsouza1234@gmail.com>
Signed-off-by: Leander Stephen D'Souza <leanderdsouza1234@gmail.com>
1fef30f to
c722074
Compare
Signed-off-by: Leander Stephen D'Souza <leanderdsouza1234@gmail.com>
c722074 to
f24838b
Compare
Thank you <3
All tests from #72 have been designed to run without an internet connection to ensure compatibility across various operating systems. So, I do not think we can add a real external fork to validate this behaviour. |
Basic Info
Description of contribution in a few bullet points
Description of how this change was tested
1. Testing against vcs2l old pull requests
First, clone the codebase:
Next, use the following
repos.yamlto get the commit hash of the following external fork:Finally, update the existing repository:
This will successfully checkout the repository to the specific hash.
This step fails without this contribution with the following error message:
=== ./vcs2l-ext-fork (git) === Could not checkout ref '4ad2dd14b0033c275e5faff7935c4a7d7581fee4': fatal: reference is not a tree: 4ad2dd14b0033c275e5faff7935c4a7d7581fee42. Test against the reproduction step in the parent issue - dirk-thomas/vcstool#258
Reproduction measures specified by @timor-raiman