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

Include commit hashes for git dependencies #122

Open
Shnatsel opened this issue May 12, 2023 · 9 comments
Open

Include commit hashes for git dependencies #122

Shnatsel opened this issue May 12, 2023 · 9 comments

Comments

@Shnatsel
Copy link
Member

Shnatsel commented May 12, 2023

Add a new commit field to track what the exact commit on which the dependency was built. It should only be included when SourceKind is git.

The final entry would look like this:

commit: "311f9932128667b8b18113becdea276b3d98aace"

@Shnatsel
Copy link
Member Author

@figsoda would this cause any issues for Nix, e.g. blow up the binary size? I understand Nix uses Git a lot in its workflows. When the package is built, does it refer to other packages locally (by path), to a git repo (i.e. git = https://example.com in Cargo.lock, or just pull them from crates.io as usual?

@jcgruenhage same question for Void Linux

@bjorn3
Copy link

bjorn3 commented May 12, 2023

Another option would be to include the tree hash instead of commit hash. This is invariant to commit message rewords and squash merges when the original commit was already based on the latest commit of the target branch. In addition it would be possible to record the tree hash for just the subdirectory of the respective crate, thus increasing the likelihood of being able to go back to the source of this specific crate when eg force pushing a change to a different crate in a mono repo.

@Shnatsel
Copy link
Member Author

That's a good point, but on the other hand I don't really want to diverge from the hash recorded in Cargo.lock and reported by cargo metadata - that seems like it would be a source of confusion.

@figsoda
Copy link
Contributor

figsoda commented May 12, 2023

@figsoda would this cause any issues for Nix, e.g. blow up the binary size? I understand Nix uses Git a lot in its workflows. When the package is built, does it refer to other packages locally (by path), to a git repo (i.e. git = https://example.com in Cargo.lock, or just pull them from crates.io as usual?

It shouldn't cause any issue afaict, the sources are vendored from crates.io in a separate derivation (package) and replaces crates.io in the actual build with this cargo config. The Cargo.lock is unmodified

[source.crates-io]
replace-with = "vendored-sources"

[source.vendored-sources]
directory = "..."

@jcgruenhage
Copy link

Shouldn't cause any trouble for Void, no. We're sourcing software from tarballs as published either by the upstream on their dev platform (such as github/gitlab/sourcehut/etc), or if that's not available from crates.io. Outside of patching dependencies in some exceptions, we're also not doing any source replacements or changing the dependencies described in the Cargo.toml, so should be perfectly fine for us :)

@eminence
Copy link

Hi all 👋

Any rough ETA when this feature might be released?

@Shnatsel
Copy link
Member Author

@eminence life happened, and I am currently occupied with other things. If you have use cases for this feature I could bump up the priority of cutting a release.

I'd appreciate if you could tell me what your use case is. I need to understand the needs of the users so that I don't accidentally break the use case in the future.

@eminence
Copy link

At work, I have binaries that depend on several internal git dependencies (we don't have an internal crate repo to use). The version number stored in the embedded manifest is not sufficient to exactly identify the version of the git dependency. So I'm interested in having the hash of the git dependency.

I thought that the implementation in #123 would solve this. However, I just did a test and I'm not actually seeing the git hashes (when extracted via the rust-audit-info tool). Here's what I'm seeing:

        {
            "name": "terminal_size",
            "version": "0.2.6",
            "source": "git",
            "dependencies": [
                140
            ]
        },

For completeness, here is how I tested:

  • Clone this repo, and checkout da85607
  • Install both tools with cargo install --path rust-audit-info and cargo install --path cargo-auditable
  • Build my binary with cargo clean then cargo auditable build --release --bin foo
  • Extract the embedded manifest with rust-audit-info .\target\release\foo.exe. I have several git dependencies. One of them is the terminal_size package, and its data is what is shown above.

@Shnatsel
Copy link
Member Author

@Adhalianna I'd appreciate if you could take a look if you're available. If not, that's fine too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants