Go: docs: contribution guidelines: add a note about using bash when configuring git

[jonogould]

Problem

• I run macOS, and my default shell is configured as zsh
• When attempting to copy and paste the script from the textbox on the "Configure git" page (https://www.googlesource.com/new-password) I get a shell error stating that history and tr are not found
• When I exec bash and run the script again, it works perfectly

Suggested solution

On https://tip.golang.org/doc/contribute.html#config_git_auth, under section Step 2: Configure git authentication, there are 3 list items.

The 3rd item in the list states the following:

1. Copy and run this script locally in your command line terminal to store your secret authentication token in a .gitcookies file. If you are using a Windows computer and running cmd, you should instead follow the instructions in the yellow box to run the command; otherwise run the regular script.

Perhaps we can add a note about switching to bash before running the script, if the user happens to be configured to a different shell?

[mvdan]

/cc @rasky

[agnivade]

I don't see this as a Go documentation issue. We are just mirroring what is mentioned on https://www.googlesource.com/new-password.

While it is fine to make the change, I think the script itself should be modified to run on most common shells. Or if that is not feasible, then the instructions should be changed on that page first. And then reflected on golang.org.

[gopherbot]

Change https://golang.org/cl/117177 mentions this issue: doc: add note for non-bash users to contribute.html

[mvdan]

@agnivade it does look like the current script is fairly Bash-specific. For example, on another POSIX Shell, dash:

$ set -o history
dash: 4: set: Illegal option -o history

The rest of the script, including tr and the heredoc, look portable enough.

[mvdan]

By which I mean - if we want to fix this by changing the shell code that Gerrit gives, that also seems like a good option to me. We would have to figure out what that would look like, though.

[agnivade]

By which I mean - if we want to fix this by changing the shell code that Gerrit gives, that also seems like a good option to me.

Yea that's what I would prefer.

A tangential question though - What are we gaining by doing this "disable bash history" business ? Is it a security issue ? Are there more portable ways of disabling shell history ?

[mvdan]

@bradfitz @ianlancetaylor do we know where the source for https://www.googlesource.com/new-password is, or exactly why the history options are necessary? Ideally, the fix would be there to make the script portable, instead of telling our contributors to use Bash.

For example, I imagine that replacing set +o history with set +o history || true should work, to only disable the history if that is possible. Although if the script really cares about security, perhaps it's best to tell the user to use Bash on the googlesource page.

[bradfitz]

@dborowitz and team own that page.

[dborowitz]

I filed an internal bug to track this.

[dborowitz]

We've made some changes to the new-password page which should roll out next week:

• Tweaked the history options to work in zsh as well as bash. (Unfortunately, there is no actual portable way to suppress history recording that we could find.)
• Added the sentence: "This script is designed to work in bash and zsh. It ensures that the password is not saved in your shell history."

Thanks for the report.

[jonogould]

Thanks @dborowitz, and team, for the quick response.
The outcome of this simple doc request is incredible to witness, and a testament to this community 🎉

[myitcv]

@dborowitz - drive-by question here, but does temporarily unsetting (then resetting) HISTFILE not work in both shells, and indeed others?

[dborowitz]

I didn't personally do the investigation, but here's what Jonathan Nieder
said about HISTFILE:

"This is pretty widely portable (to bash, zsh, and ksh) but only affects
what happens when history is flushed to disk; it doesn't affect the history
shown before then with "fc -l". Since HISTFILE can change after our command
sequence and before history is flushed, this is not reliable or intuitive
enough to count on."

On Fri, Jun 22, 2018, 09:53 Paul Jolly [e-mail] wrote:

@dborowitz https://github.com/dborowitz - drive-by question here, but
does temporarily unsetting (then resetting) HISTFILE not work in both
shells, and indeed others?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/golang/go/issues/25788#issuecomment-399449622, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAL5bwFOw7KyYRWnHEwPumUQpFcANKbZks5t_PbYgaJpZM4Ue9ZW
.

[dborowitz]

For those following along at home, the changes have been rolled out.

This may have not been clear from my prior comment, but this update also included a change to the script itself to ensure that it is indeed compatible with zsh:

set +o history 2>/dev/null || setopt HIST_IGNORE_SPACE 2>/dev/null
 touch ~/.gitcookies
# etc.

[agnivade]

Awesome, thanks.

Going to close this issue since I believe the documentation from our side is correct now -

Copy and run this script locally in your terminal to store your secret authentication token in a .gitcookies file.

Explicitly mentioning the valid shells to run on (bash,zsh) might be overkill when it is already mentioned on the googlesource page. Feel free to comment if you disagree.