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

fetchgitLocal uses an incorrect way to get the git directory #10873

Closed
ftrvxmtrx opened this issue Nov 7, 2015 · 18 comments
Closed

fetchgitLocal uses an incorrect way to get the git directory #10873

ftrvxmtrx opened this issue Nov 7, 2015 · 18 comments
Labels
0.kind: bug Something is broken 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md

Comments

@ftrvxmtrx
Copy link
Member

7266040#diff-c7bec12494f11831828500ff286831d8R13

The ROOT=... logic should work for most of the repos, but not for submodules, which have .git as a file with a gitdir parameter pointing somewhere else.
I propose to change the above logic to something like:

gitdir=$(git rev-parse --git-dir)

cp $gitdir/index $gitdir/index-user
...

The other problem is that it's still not enough to make nix-build -A my-subpackage work again, for example:

cp: cannot create regular file '/home/..../.git/modules/my-submodule/index-user': Permission denied

Until this is fixed I have to put an old version of fetchgitLocal into my projects with submodules as Nix packages.

@ftrvxmtrx
Copy link
Member Author

ping @Ericson2314

@Ericson2314
Copy link
Member

Ah! So I forgot that the git repo I tested it with is on an NTFS partition, and the FUSE NTFS driver doesn't support/enforce permissions. That means we either need to copy the whole git repo (or at least .git) to somewhere we have write access, or just go with what's currently in the index. Yuck. Which option is more palatable to you?

Good catch and good solution re submodules. I wish that command returned an absolute path, but that is not necessary by any means. Perhaps we should also take an optional arguments for specifying the git directory and working directory, as git allows those to be independently located.

[Finally I'd like to get some code reuse going between the second derivation and fetchgit, as once the .git directory is ready, I'd imagine they both want to basically do the same thing with it. But that is a longer term project.]

Sorry again for this inconvenience.

@Ericson2314
Copy link
Member

@pchiusano ^ bad news. I think I'm going to need revert to the old behavior. Still note needs just updating the index, not a full commit.

@ftrvxmtrx
Copy link
Member Author

@Ericson2314 I would prefer to go with only what's in the index, actually.

@Ericson2314
Copy link
Member

Ok, I'll make a PR.

@Ericson2314
Copy link
Member

Ericson2314@0e3a503

I got to go, if you could test this that would be great

@ftrvxmtrx
Copy link
Member Author

Unfortunately, it didn't work:

building path(s) ‘/nix/store/ydashigqbyrid84nmg8r3hpsyxgm64sa-put-in-git’
fatal: Not a git repository (or any of the parent directories): .git
these derivations will be built:
  /nix/store/xn5hd37cz841v202zssp0hfc38zpk5yb-put-in-nix.drv
building path(s) ‘/nix/store/1x2gvwnzzrra3iagy2h1m57ji48sanna-put-in-nix’
usage: git archive [<options>] <tree-ish> [<path>...]
   or: git archive --list
   or: git archive --remote <repo> [--exec <cmd>] [<options>] <tree-ish> [<path>...]
   or: git archive --remote <repo> [--exec <cmd>] --list

    --format <fmt>        archive format
    --prefix <prefix>     prepend prefix to each pathname in the archive
    -o, --output <file>   write the archive to this file
    --worktree-attributes
                          read .gitattributes in working directory
    -v, --verbose         report archived files on stderr
    -0                    store only
    -1                    compress faster
    -9                    compress better

    -l, --list            list supported archive formats

    --remote <repo>       retrieve the archive from remote repository <repo>
    --exec <command>      path to the remote git-upload-archive command

tar: This does not look like a tar archive
tar: Exiting with failure status due to previous errors
....

@Ericson2314
Copy link
Member

Ah, I forgot about the git inside the $(...)$ which didn't get the -C flag. I'll just go back to changing directories then.

@Ericson2314
Copy link
Member

OK see master...Ericson2314:fetchgitLocal and the division of labor of each commits. Though I don't think this will actually fix your problem as the git write-tree needs write access to .git---and that is the backbone of my original change :(.

Furthermore, even the original fetchgitlocal (before any of my changes) breaks if the build user doesn't have read access. This makes me think we really should be running the first derivation under the current user. Not sure how to do this, maybe https://github.com/shlevy/nix-exec ? It's do that, or copy the entire repo into the store and then prune the junk.

@Ericson2314
Copy link
Member

Another temporary fix is to give the nixbld group (the build users) group permission on your repo:

  • make nixbld the group on everything
  • give nixbld read on everything, and execute on all directories
  • give nixbuild write on .git and children
  • remove sticky bit on .git and children

This is obviously an unsatisfactory long term solution. On a multi user system with mutually untrusted users with the power to use nix, this is unsafe as other users could write malicious derivations to mess with your repo.

@Fuuzetsu
Copy link
Member

hit on current master

building path(s) ‘/nix/store/s6ygcb7h9qbl43ak5sqfv4rkqrg204n4-put-in-git’
cp: cannot create regular file '.git/index-user': Permission denied

@joachifm joachifm added the 0.kind: bug Something is broken label Apr 3, 2016
@neilmayhew
Copy link
Member

I think a better and more functional way of doing it is to put a git-archive export of HEAD into the store, and then generate a patch from a git diff HEAD (which also gets put into the store). This involves no changes to the .git directory, not even temporary ones.

Also, the current method of using git add . can bring a lot of irrelevant crud (eg test data) into the git repo and also into the store. Putting HEAD into the store potentially avoids a lot of churn since presumably the developer is making changes in the working directory but not committing them while they get something working. The patches will change, but they're only small. The majority of the source files won't be changing, so the git export in the store will have the same hash and won't need to get recreated.

Diffing against HEAD doesn't pick up new files, so a possible refinement is to generate two patches, one between HEAD and the index (ie git diff --cached) and one between the index and the working directory (ie git diff). If the developer has a new file, they can add it to the index with git add -A, which will make it show up in the diffs.

@teto
Copy link
Member

teto commented Nov 17, 2017

fetchgitLocal would be useful in so many cases but it's not usable now because of the permission problem /tmp/nix-build-put-in-git.drv-0/.attr-0: line 1: cd: /home/teto/mptcp: Permission denied
I am so looking forward for a fix @Ericson2314 . @neilmayhew solution looks good.

@teto
Copy link
Member

teto commented Nov 17, 2017

Wouldn't it be possible to copy the .git folder in/tmp and export $GIT_DIR before the rest of the operations ?

@srghma
Copy link
Contributor

srghma commented Apr 14, 2018

based on #31363 (comment) can we remove fetchGitLocal in preference of builtins.fetchGit?

@neilmayhew
Copy link
Member

@srghma fetchGit ./. isn't the same as fetchGitLocal: it uses only HEAD and doesn't include the state of the working tree. See my comments above for an explanation of the use case.

@Ericson2314
Copy link
Member

git diff HEAD or git diff --staged + builtins.fetchgit sounds like the way to go!

@stale
Copy link

stale bot commented Jun 4, 2020

Thank you for your contributions.

This has been automatically marked as stale because it has had no activity for 180 days.

If this is still important to you, we ask that you leave a comment below. Your comment can be as simple as "still important to me". This lets people see that at least one person still cares about this. Someone will have to do this at most twice a year if there is no other activity.

Here are suggestions that might help resolve this more quickly:

  1. Search for maintainers and people that previously touched the related code and @ mention them in a comment.
  2. Ask on the NixOS Discourse.
  3. Ask on the #nixos channel on irc.freenode.net.

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 4, 2020
@teto teto closed this as completed Jun 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0.kind: bug Something is broken 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md
Projects
None yet
Development

No branches or pull requests

7 participants