obi1kenobi / cargo-semver-checks

Scan your Rust crate for semver violations.
Apache License 2.0
1.13k stars 71 forks source link

Add support for repositories with git submodules #927

Open spearman opened 5 days ago

spearman commented 5 days ago

Steps to reproduce the bug with the above code

I have a crate that includes several workspace members as git submodules in the repository. I want to check if I need to bump the version before committing.

Actual Behaviour

I run:

cargo semver-checks --baseline-rev <rev>

This appears to clone the revision into the semver-checks target directory, but it hasn't cloned any of the workspace submodules, so it fails during cargo update with failed to load source for dependency and gives a path to the submodule manifest that doesn't exist because it hasn't been initialized.

Is there a way to make this work?

Expected Behaviour

Finishes running without error.

Generated System Information

System information:

Software version

cargo-semver-checks 0.35.0

Operating system

Linux 6.1.96

Command-line

/home/spearman/.cargo/bin/cargo-semver-checks semver-checks --bugreport 

cargo version

> cargo -V 
cargo 1.82.0-nightly (0d8d22f83 2024-08-08)

Compile time information

Cargo build configuration:

[net] git-fetch-with-cli = true

Build Configuration

No response

Additional Context

No response

obi1kenobi commented 5 days ago

Interesting, we've never had to work with submodules before so this wasn't even on our radar. Thanks for opening this issue!

If I understood correctly, this is an issue with any argument passed to --baseline-rev, right? If so, I think we'll probably be able to put together a concise repro case we can check into our test suite here.

spearman commented 5 days ago

If I understood correctly, this is an issue with any argument passed to --baseline-rev, right?

It happens with any revision passed to --baseline-rev, if there are other arguments I'm not aware of them.

I think the git clone command would just need to include --recurse-submodules.

obi1kenobi commented 2 days ago

@Byron if you have a sec, I'd love your help. I think you kindly contributed our extract_tree() function for checking out a revision in #531. It seems like our original git2 implementation never updated submodules, and the gitoxide one doesn't either.

I wasn't able to figure out how to update the extract_tree() function to update submodules. I'm not that familiar with git internals, so perhaps I've missed something that was obvious from the docs — my apologies if so. I'd appreciate any help with making this work!

Byron commented 2 days ago

Thanks for letting me know! After taking a quick look at extract_tree() I think It probably will take more to make this work. My thoughts are that first of all, the original repository needs to be cloned with submodules. I don't know if it performs a bare clone or not, but it also shouldn't matter much. With gix::Repository::submodules() that can already be accomplished today.

From there one has to see how cargo-semver-check wants to access trees within submodules - maybe the expectation here is that it just recurses into them. This would also be possible as the aforementioned function would allow you to open a submodule repository and one could extract the tree from there.

Unfortunately I can't implement it for you, but believe that with that function it will be possible to discover submodules and either see their URLs to clone from, or open their repos to extract objects from their clones. Clones would be interesting as an active submodule would then need to register itself in the git configuration, and that's not easily done yet with gix, but it would be easy with git2.

The easiest way to clone with submodules would be to use the git binary, if that's an option at all - maybe it could be a special case for a while so that it is used to recursively init submodules if submodules are detected after the initial clone. That way, it's trivial to get the clone done, and git would only be used if there are submodules so the standard path isn't less portable.

For executing git, there is gix::path::env::exe_invocation() to obtain a valid git executable - that's particularly useful on Windows where it might not be in the PATH.

Here is the summary of what I would do to keep it simple:

I hope that helps.

obi1kenobi commented 2 days ago

Thank you for your prompt reply @Byron, I appreciate it!

@spearman it seems this would unfortunately take a decent bit of work to support, and isn't the easy fix we were hoping it would be. I'd happily take a well-tested PR for it, but I'm afraid I can't prioritize building it myself at the moment. I'm sorry — I know that isn't the answer you were hoping for.

If the use case that needs this functionality is of a commercial nature, I'd be happy to chat about possible options there.