Skip to content
This repository has been archived by the owner on Sep 11, 2020. It is now read-only.

Add .gitconfig support #760

Open
maguro opened this issue Feb 26, 2018 · 16 comments · May be fixed by #1243
Open

Add .gitconfig support #760

maguro opened this issue Feb 26, 2018 · 16 comments · May be fixed by #1243

Comments

@maguro
Copy link
Contributor

maguro commented Feb 26, 2018

Not sure if a first class object to represent the .gitconfig file should be provided or if the global config information should be squirreled away inside config.Config. There is the ~/.gitignore_global file to consider as well.

It seems to me that a first class object and storer interface is the way to go. These are real physical entities and they should have their counterparts in the config package.

@orirawlings
Copy link
Contributor

orirawlings commented Feb 28, 2018

@maguro,

To clarify, are you referring to the system and global git configuration files (i.e. the ones accessed in git config via the --system and --global options)?

From git help config on my local OS X:

--global
           For writing options: write to global ~/.gitconfig file rather than the repository .git/config, write to $XDG_CONFIG_HOME/git/config file if this file
           exists and the ~/.gitconfig file doesn't.

           For reading options: read only from global ~/.gitconfig and from $XDG_CONFIG_HOME/git/config rather than from all available files.

           See also the section called "FILES".

--system
           For writing options: write to system-wide $(prefix)/etc/gitconfig rather than the repository .git/config.

           For reading options: read only from system-wide $(prefix)/etc/gitconfig rather than from all available files.

           See also the section called "FILES".

--local
           For writing options: write to the repository .git/config file. This is the default behavior.

           For reading options: read only from the repository .git/config rather than from all available files.

           See also the section called "FILES".

@orirawlings
Copy link
Contributor

orirawlings commented Feb 28, 2018

I think the config package is meant to be somewhat agnostic of what files are used to store the configuration (or even if files are used at all).

Currently there code to pull the --local .git/config file lives in the dotgit package. This is then wrapped up by the storage/filesystem.ConfigStorage implementation.

We probably do want to modify config somehow so that multiple Configs could be composed together (with more specific configs override settings in less specific configs).

@maguro
Copy link
Contributor Author

maguro commented Mar 5, 2018

I am speaking of the global and system configurations, which are not currently being read.

I did read through the config code. It seemed to me that it was rife with mixed concerns. I don't see how to cleanly accommodate, with parity, system and global configuration files without backward incompatible changes. More specifically, the marshaling code seems to assume that only a local config environment is in place; I hold a poor opinion about classes that have ser/der member functions.

Maybe we could have three internal structs, Local, Global, and System; making them "top-level" structs which would be fields of Config would have a number of advantages. The go-git code base would still get properties from the parent Config struct and the accessors would effect proper precedence. Such accessors could properly hunt down global ignores; the impetus for my endeavor.

The troublesome marshaling code would be limited to the local config file. I have no problem with that since anyone wanting to programmatically modify the global and system configurations could use some out of band mechanism. If such functionality were desired, things could be done slightly differently. Using io.Reader and io.Writer may be better than byte[]. Instances of Global and System would be passed in as a clone/open options. The ser/der bits for Global and System would be separate sets utility functions.

I don't see options for Plain*(). Maybe PlainOpenWith() and PlainInitWith() could be added. I think that OpenWith() and InitWith() would need to be added as well.

@maguro
Copy link
Contributor Author

maguro commented Mar 5, 2018

Actually, I'm quite fond of something like:

Open(s storage.Storer, worktree billy.Filesystem, opts ...RepositoryOption)

Then one could make calls such as

r, err := git.Open(s, workgree, WithGlobalConfig(), WithSystemConfig())

@maguro
Copy link
Contributor Author

maguro commented Mar 8, 2018

Can I get a 👍 from a Spaniard? :)

Is there anything you all would like to see before I start tinkering with this?

@maguro
Copy link
Contributor Author

maguro commented Mar 10, 2018

I'm going to start tinkering with this. Not sure when it will get done so if someone wants to work on this, feel free to ping me.

@mcuadros
Copy link
Contributor

@maguro sorry we were the last week at Git Merge conference, you approach make sense.

About the Open function maybe we can create a new function call OpenWithOptions(*OpenOptions) since for sure we require more options, like: #765

@maguro
Copy link
Contributor Author

maguro commented Mar 14, 2018

To me

r, err := git.Open(s, worktree, WithGlobalConfig(), WithSystemConfig())

seems more elegant. If you don't want to use options you get the backwards compatible

r, err := git.Open(s, worktree)

@maguro
Copy link
Contributor Author

maguro commented Mar 20, 2018

What's the consensus on this?

@orirawlings
Copy link
Contributor

For consistency with other API methods we should probably stick with the OpenOptions approach.

@maguro
Copy link
Contributor Author

maguro commented Mar 22, 2018

Yep. I will.

@InTheCloudDan
Copy link

Is there any additional work being done on support the user's global gitconfig? I'm not sure where to even get started on it exactly but if someone could provide some guidance I'd like to take a look at it.

@maguro
Copy link
Contributor Author

maguro commented Jun 6, 2018

I don't plan on it. It seems to me that the way go-git is currently architected makes it difficult. You're welcome to take a stab at it.

@StevenACoffman
Copy link

Ref #603 : having any globally ignored files cause workTree.Status().IsClean() to report false, when git status --porcelain shows it is actually clean. This is a breadcrumb for similar searches.

@StevenACoffman
Copy link

StevenACoffman commented Aug 20, 2019

I am not sure how to use the #825 convenience methods:

	w, err := repo.Worktree()
	CheckIfError(err)
	gp, err := gitignore.LoadGlobalPatterns(w.Filesystem)
	CheckIfError(err)
	w.Excludes = append(w.Excludes, gp...)
	sp, err := gitignore.LoadSystemPatterns(w.Filesystem)
	CheckIfError(err)
	w.Excludes = append(w.Excludes, sp...)

	status, err := w.Status()
	CheckIfError(err)
	fmt.Println(status)

	fmt.Println(status.IsClean())

I still get globally ignored files polluting the worktree status:

?? .idea/workspace.xml
?? cmd/generate-tls-cert/.gitignore
?? .idea/misc.xml
?? .idea/modules.xml
?? .idea/toolbox.iml
?? .idea/vcs.xml

false

@StevenACoffman
Copy link

Workaround gist is here but this is excerpt:

	w, err := repo.Worktree()
	CheckIfError(err)

	cfg, err := parseGitConfig()
	if err != nil {
		fmt.Fprintln(os.Stderr, "Could not read ~/.gitconfig: "+err.Error())
		return
	}

	excludesfile := getExcludesFile(cfg)
	if excludesfile == "" {
		fmt.Fprintln(os.Stderr, "Could not get core.excludesfile from ~/.gitconfig")
		return
	}

	ps, err := parseExcludesFile(excludesfile)
	CheckIfError(err)
	w.Excludes = append(ps, w.Excludes...)

	status, err := w.Status()
	CheckIfError(err)
	fmt.Println(status)

	fmt.Println(status.IsClean())

}

// CheckIfError should be used to naively panics if an error is not nil.
func CheckIfError(err error) {
	if err == nil {
		return
	}

	fmt.Printf("\x1b[31;1m%s\x1b[0m\n", fmt.Sprintf("error: %s", err))
	os.Exit(1)
}

// Info should be used to describe the example commands that are about to run.
func Info(format string, args ...interface{}) {
	fmt.Printf("\x1b[34;1m%s\x1b[0m\n", fmt.Sprintf(format, args...))
}

func parseGitConfig() (*config.Config, error) {
	cfg := config.NewConfig()

	usr, err := user.Current()
	CheckIfError(err)

	b, err := ioutil.ReadFile(usr.HomeDir + "/.gitconfig")
	if err != nil {
		return nil, err
	}

	if err := cfg.Unmarshal(b); err != nil {
		return nil, err
	}

	return cfg, err
}

func getExcludesFile(cfg *config.Config) string {
	for _, sec := range cfg.Raw.Sections {
		if sec.Name == "core" {
			for _, opt := range sec.Options {
				if opt.Key == "excludesfile" {
					return opt.Value
				}
			}
		}
	}
	return ""
}

func parseExcludesFile(excludesfile string) ([]gitignore.Pattern, error) {
	excludesfile, err := expandTilde(excludesfile)
	if err != nil {
		return nil, err
	}

	data, err := ioutil.ReadFile(excludesfile)
	if err != nil {
		return nil, err
	}

	var ps []gitignore.Pattern
	for _, s := range strings.Split(string(data), "\n") {
		if !strings.HasPrefix(s, "#") && len(strings.TrimSpace(s)) > 0 {
			ps = append(ps, gitignore.ParsePattern(s, nil))
		}
	}

	return ps, nil
}

// "~/.gitignore" -> "/home/tyru/.gitignore"
func expandTilde(path string) (string, error) {
	if !strings.HasPrefix(path, "~") {
		return path, nil
	}
	var paths []string
	u, err := user.Current()
	if err != nil {
		return "", err
	}
	for _, p := range strings.Split(path, string(filepath.Separator)) {
		if p == "~" {
			paths = append(paths, u.HomeDir)
		} else {
			paths = append(paths, p)
		}
	}
	return "/" + filepath.Join(paths...), nil
}

djgilcrease pushed a commit to djgilcrease/go-git that referenced this issue Nov 15, 2019
@djgilcrease djgilcrease linked a pull request Nov 15, 2019 that will close this issue
djgilcrease pushed a commit to djgilcrease/go-git that referenced this issue Nov 15, 2019
Closes src-d#760

Signed-off-by: Dj Gilcrease <[email protected]>
djgilcrease pushed a commit to djgilcrease/go-git that referenced this issue Nov 15, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants