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

Ignore .sock files in git.WorkTree #312

Closed
gleich opened this issue May 10, 2021 · 11 comments
Closed

Ignore .sock files in git.WorkTree #312

gleich opened this issue May 10, 2021 · 11 comments
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@gleich
Copy link

gleich commented May 10, 2021

.sock files should be ignored when getting the working tree as they cause the following error:

open /Users/matt/src/hackclub/private/go/hack-as-a-service/dokkud_sock/dokkud.sock: operation not supported on socket
@steffakasid
Copy link

Same issue here:
open /Users/sid/.gnupg/S.gpg-agent.browser: operation not supported on socket would need to exclude this as well. No idea how to do it.

@steiler
Copy link
Contributor

steiler commented Nov 15, 2023

I have a syslog-ng.ctl linux socket file. So ignoring based on file extension is not the overall solution.
The file is also mentioned in the .gitignore but is anyways evaluated in the merkletrie.DiffTree(from, to, diffTreeIsEquals).
Not sure if it would make sense to take gitignore into consideration when diffing.

@steiler
Copy link
Contributor

steiler commented Nov 15, 2023

So the issue lies further down the call stack.
Right here, there is a special case for symlinks but some specifics for fs.ModeSocket // S: Unix domain socket (https://pkg.go.dev/io/fs#FileMode) is missing right here:
https://github.com/go-git/go-git/blob/master/utils/merkletrie/filesystem/node.go#L149-L153

@steiler
Copy link
Contributor

steiler commented Nov 15, 2023

Commandline git seems to fully ignore the file.
Can someone assist / tell me where to hook in to ignore these type of files go-git? I'd be willing to craft a PR.

mava@server01:~/projects/containerlab3/srl-telemetry-lab/configs/syslog$ git status
On branch main
Your branch is ahead of 'origin/main' by 1 commit.
  (use "git push" to publish your local commits)

nothing to commit, working tree clean
mava@server01:~/projects/containerlab3/srl-telemetry-lab/configs/syslog$ touch myfile
mava@server01:~/projects/containerlab3/srl-telemetry-lab/configs/syslog$ git add myfile syslog-ng.ctl
mava@server01:~/projects/containerlab3/srl-telemetry-lab/configs/syslog$ git status
On branch main
Your branch is ahead of 'origin/main' by 1 commit.
  (use "git push" to publish your local commits)

Changes to be committed:
  (use "git restore --staged <file>..." to unstage)
        new file:   myfile

@hellt
Copy link

hellt commented Nov 16, 2023

That would be great to resolve as we hit this as well. And workarounds are not always possible it seems

@pjbgf pjbgf added bug Something isn't working help wanted Extra attention is needed labels Nov 16, 2023
@pjbgf
Copy link
Member

pjbgf commented Nov 16, 2023

@steiler I think you are on the right direction - we need to handle ModeSocket differently. We just need to confirm exactly what upstream does. My assumption is that they would hash that file type with empty content.

@steiler
Copy link
Contributor

steiler commented Nov 16, 2023

Would it be sufficient to adjust just the hashing behaviour? I'm thinking if the file is not there at all there might be a different execution path that might still try to load that file. But thats my wild imagination on how it might work?!
But yes would be good if someone with sufficient knowledge / insight could confirm the upstream behaviour.

@steiler
Copy link
Contributor

steiler commented Nov 16, 2023

@pjbgf tried catching the ModeSocket in calculateHash(...) as here:
b7aa82e

The error I get now is:
Error: to: no equivalent git mode for Srwxr-xr-x

Originating from here:

// NewFromOSFileMode returns the FileMode used by git to represent
// the provided file system modes and a nil error on success. If the
// file system mode cannot be mapped to any valid git mode (as with
// sockets or named pipes), it will return Empty and an error.
//
// Note that some git modes cannot be generated from os.FileModes, like
// Deprecated and Submodule; while Empty will be returned, along with an
// error, only when the method fails.
func NewFromOSFileMode(m os.FileMode) (FileMode, error) {
if m.IsRegular() {
if isSetTemporary(m) {
return Empty, fmt.Errorf("no equivalent git mode for %s", m)
}
if isSetCharDevice(m) {
return Empty, fmt.Errorf("no equivalent git mode for %s", m)
}
if isSetUserExecutable(m) {
return Executable, nil
}
return Regular, nil
}
if m.IsDir() {
return Dir, nil
}
if isSetSymLink(m) {
return Symlink, nil
}
return Empty, fmt.Errorf("no equivalent git mode for %s", m)
}

Which is basically the next call after retrieving the hash. I somewhat have the feeling the Sockets have to be filtered out earlier since calculateHash()

func (n *node) calculateHash(path string, file os.FileInfo) ([]byte, error) {

does need to return a byte slice and the calling newChildNode(...) would also error out or add the child to the children slice.
func (n *node) newChildNode(file os.FileInfo) (*node, error) {

@pjbgf
Copy link
Member

pjbgf commented Nov 16, 2023

The git CLI does not seem to even accept such files:

$ cd /tmp
$ mkdir test-sock
$ cd test-sock
$ git init
$ python3 -c "import socket as s; sock = s.socket(s.AF_UNIX); sock.bind('/tmp/test-sock/sock-et')"
$ mkfifo test-fifo
$ git add --force test-fifo
$ git add --force sock-et
$ git commit -m "Add files"

On branch master

Initial commit

nothing to commit (create/copy files and use "git add" to track)

If I stage a regular file, then override it with a socket, it errors:

$ touch another-file
$ git add another-file
$ rm another-file
$ python3 -c "import socket as s; sock = s.socket(s.AF_UNIX); sock.bind('/tmp/test-sock/another-file')"
$ git add .
error: another-file: can only add regular files, symbolic links or git-directories
fatal: updating files failed

Here's the upstream code for that specific validation.

Maybe the change you are looking for is a continue here for those types of files. Additionally, a validation will need to be added when folks are trying to amend staged files with invalid types.

@steiler
Copy link
Contributor

steiler commented Nov 16, 2023

Right, thats exactly what I did now here:
master...steiler:go-git:fixSockets

That does the trick for our usecase. However I'm not sure about further side effects.

Would you advise to open this as a PR?

@pjbgf
Copy link
Member

pjbgf commented Nov 16, 2023

Feel free to open the PR and tag me for a review. Please add some tests to avoid future regressions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

5 participants