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

vcpkg: make builtin-baseline work #328937

Closed

Conversation

Jayman2000
Copy link
Contributor

Description of changes

Consider this vcpkg.json file:

{
  "dependencies": [ "fmt", "zlib" ],
  "builtin-baseline": "101ae1f63a22d262f80a68034a1550da5dfdd05f"
}

At the moment, if you try to run “vcpkg install” with that vcpkg.json file, then vcpkg will fail with this error:

error: while checking out baseline from commit '101ae1f63a22d262f80a68034a1550da5dfdd05f', failed to `git show` versions/baseline.json. This may be fixed by fetching commits with `git fetch`.
error: git failed with exit code: (128).
fatal: not a git repository: '.git'
while checking out baseline 101ae1f63a22d262f80a68034a1550da5dfdd05f

This PR fixes that problem.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@gracicot
Copy link
Contributor

Before merging this I'd like to experiment with another solution first that may not involve having vcpkg living in its own git repo.

Quick question: does this baseline support allow specifying any arbitrary baseline?

Consider this vcpkg.json file:

  {
    "dependencies": [ "fmt", "zlib" ],
    "builtin-baseline": "101ae1f63a22d262f80a68034a1550da5dfdd05f"
  }

Before this change, if you tried to run “vcpkg install” with that
vcpkg.json file, then vcpkg would fail with this error:

  error: while checking out baseline from commit '101ae1f63a22d262f80a68034a1550da5dfdd05f', failed to `git show` versions/baseline.json. This may be fixed by fetching commits with `git fetch`.
  error: git failed with exit code: (128).
  fatal: not a git repository: '.git'
  while checking out baseline 101ae1f63a22d262f80a68034a1550da5dfdd05f

This commit fixes that problem.
@Jayman2000 Jayman2000 force-pushed the make-vcpkg-builtin-baseline-work branch from 52be313 to 79e11b3 Compare July 21, 2024 23:52
@Jayman2000
Copy link
Contributor Author

Quick question: does this baseline support allow specifying any arbitrary baseline?

No. At the moment, the vcpkg package’s version is 2024.06.15 which corresponds to commit f7423ee180c4b7f40d43402c2feb3859161ef625. That means that you can use f7423ee180c4b7f40d43402c2feb3859161ef625 or any of its ancestors as the baseline. You can’t use any commits that are newer than that one, and you can’t use any commits that aren’t in vcpkg’s master branch.

@ofborg ofborg bot requested a review from h7x4 July 22, 2024 01:02
@h7x4
Copy link
Member

h7x4 commented Jul 22, 2024

This is definitely an improvement. Earlier, I have had to manually override the source to fetch vcpkg with the correct baseline, and patch out the baseline to make it build in a sandbox

However, I am a bit worried about #8567. Have you made any considerations about it?

@gracicot
Copy link
Contributor

@h7x4 considering that you have to put a specific baseline hash for the built-in baseline to work, I don't think it would improve the situation all that much as you'll probably have to patch the baseline hash instead, or revert back to a git baseline instead.

Unless I haven't understood the use case correctly?

@h7x4
Copy link
Member

h7x4 commented Jul 22, 2024

@gracicot: The point I was trying to make is that I have had to make workarounds before, to fix the exact issue that this PR is trying to fix.

However, I'm still worried about the way this is fixed. Are there going to be issues with reproducibility and the .git directory? Possibly sudden hash mismatches?

I've played around with the idea of creating bespoke tarballs of the vcpkg releases that contains the .git directory (The GitHub source tarballs doesn't provide .git), and use them as the source for vcpkg to work around the issue. It would be nice if it could be put under an official nix organization, and automated with CI somehow. It feels like a dirty solution, but if the deepClone turns out to be problematic, I don't see any better way that doesn't require patching the inner workings of vcpkg.

@Jayman2000
Copy link
Contributor Author

However, I am a bit worried about #8567. Have you made any considerations about it?

OK, so I read through that thread and here are my thoughts. It seems like vcpkg is just designed in a bad way. The vcpkg-tool already downloads the source code for the things that it builds. It should also be able to download the vcpkg repo if it’s not already present. Nix already does this when using channels or a flake.lock file. The vcpkg-tool should do the same thing.

With that in mind, here’s what I’m thinking. Instead of making Nixpkgs’s current vcpkg package support builtin-baseline, create a new package named vcpkg-auto-clone (feel free to suggest a better name). When you install vcpkg-auto-clone, it will add binary named vcpkg to your PATH. Here’s what that binary will do:

if (~/.vcpkg-auto-clone does not exist)
    git clone https://github.com/microsoft/vcpkg ~/.vcpkg-auto-clone
if (~/.vcpkg-auto-clone/vcpkg does not exist)
    ln -s ${pkgs.vcpkg-tool}/bin/vcpkg ~/.vcpkg-auto-clone/vcpkg
run ~/.vcpkg-auto-clone/vcpkg

This vcpkg-auto-clone script would allow the vcpkg-tool to be located in a Git repo like it wants to be, but it wouldn’t put that Git repo in the Nix store.

Once we get the vcpkg-auto-clone script working, then ideally we would get upstream to fix the vcpkg-tool so that it can download the vcpkg repo automatically.


However, I'm still worried about the way this is fixed. Are there going to be issues with reproducibility and the .git directory? Possibly sudden hash mismatches?

It looks like that has already happened. “vcpkg, vcpkg.passthru.tests on aarch64-linux” succeeded, but “vcpkg, vcpkg.passthru.tests on x86_64-linux” failed because of a hash mismatch.

@smancill
Copy link
Contributor

Closed by #329939

@smancill smancill closed this Aug 16, 2024
@Jayman2000 Jayman2000 deleted the make-vcpkg-builtin-baseline-work branch August 16, 2024 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants