Skip to content

Add a test case where fetchGit is failing to cache due to packed-refs#13456

Merged
roberth merged 2 commits intoNixOS:masterfrom
fzakaria:git-pack-ref-cache
Jul 23, 2025
Merged

Add a test case where fetchGit is failing to cache due to packed-refs#13456
roberth merged 2 commits intoNixOS:masterfrom
fzakaria:git-pack-ref-cache

Conversation

@fzakaria
Copy link
Contributor

@fzakaria fzakaria commented Jul 11, 2025

builtins.fetchGit is not using the cached Git directory if packed-references are used.
This is because the ref file for the fetchGit refs/heads/master is used to check the mtime for whether to cache or not.

Let's at least codify this failure in a test case.
For now the test case expects failure but we should change it once the issue is resolved.


Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

@fzakaria fzakaria requested a review from edolstra as a code owner July 11, 2025 22:51
@github-actions github-actions bot added the with-tests Issues related to testing. PRs with tests have some priority label Jul 11, 2025
@xokdvium xokdvium force-pushed the git-pack-ref-cache branch from 5a4f674 to c83c9bf Compare July 15, 2025 21:29
Copy link
Contributor

@xokdvium xokdvium left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Yeah, the localRefFile really needs to be handled in a better way. For now having this deficiency documented in tests is a good start.

fzakaria added 2 commits July 22, 2025 02:52
builtins.fetchGit is not using the cached Git directory if
packed-references are used.

This is because the ref file for the fetchGit `refs/heads/master` is
used to check the mtime for whether to cache or not.

Let's at least codify this failure in a test case.
@xokdvium xokdvium force-pushed the git-pack-ref-cache branch from c83c9bf to 0c32b0c Compare July 21, 2025 23:52
Copy link
Member

@roberth roberth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Helps to identify the issue.

Probably caused by explicitly reading the refs dir. We should use libgit2 instead. EDIT: see @xokdvium approval

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2025-07-23-nix-team-meeting-minutes-236/67114/1

@roberth roberth merged commit 3543a73 into NixOS:master Jul 23, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

with-tests Issues related to testing. PRs with tests have some priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants