-
-
Notifications
You must be signed in to change notification settings - Fork 12
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 environment variables when executing git command #14
Conversation
Me, neither. I suggest that we
Becomes
And then add a
To the options, with the current implementation as the default. I can fix the Hugo side of this, but the above would allow us to hook this into Hugo's setup for all of this (there is a fairly strict env var filter already in place). |
Sounds good to me. Do you want me to try to implement your suggestion in this PR? Or will you take over from here and close this PR? |
If you could make the changes in this PR, I can tag a release and make the Hugo changes. |
Done, please let me know if I need to change something. |
gitmap.go
Outdated
GetGitCommandFunc func(args ...string) *exec.Cmd | ||
} | ||
|
||
func NewOptions(repository, revision string) Options { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you
- Remove NewOptions
- Inside Map: Do a
if opts.GetGitCommandFunc == nil then opts.GetGitCommandFunc = the default ...
Other than that this looks good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
d570892
to
3328c12
Compare
gitmap.go
Outdated
out, err := exec.Command(gitExec, args...).CombinedOutput() | ||
func git(opts Options, args ...string) ([]byte, error) { | ||
var cmd *exec.Cmd | ||
if opts.GetGitCommandFunc != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer if we did this in the Map
func and set the value on the Options
struct; that way we don't have to think about it if we add other git operations (the opt.GetGitCommandFunc is always set).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I did not read your previous comment carefully enough.
3328c12
to
04dc645
Compare
We encountered the issue gohugoio/hugo#8627 seemingly random, until we realized that the environment variable
GIT_TRACE
was enabled for debugging.The parsing logic does not handle the additional output very well and therefore panics with the "index out of range" error.
The simplest solution would be to ignore all environment variables.
However, I'm not absolutely sure that this has no unintended side-effects.