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.

Sign up for GitHub

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

Jump to bottom

don't pass GIT_TRACE to exec.Command calls in the git package #971

Merged
merged 2 commits into from Feb 4, 2016
Merged

don't pass GIT_TRACE to exec.Command calls in the git package #971

merged 2 commits into from Feb 4, 2016

Conversation

technoweenie
Copy link
Contributor

The git package provides an api around the git command. It doesn't make sense to pass any GIT_TRACE env values to it. Here are the errors just from the tests:

$ GIT_TRACE=1 script/test git
trace git-lfs: run_command: 'git' config -l
trace git-lfs: run_command: 'git' rev-parse HEAD --symbolic-full-name HEAD
--- FAIL: TestCurrentRefAndCurrentRemoteRef (0.21s)
    assert.go:16: /Users/rick/go/src/github.com/github/git-lfs/git/git_test.go:53
    assert.go:25: ! Sha: "12:08:04.097045 git.c:348               trace: built-in: git 'show' '-s' '--format=%H" != "8263b6dfdc1ec816145ab1635d3719d54bfa8d67"
trace git-lfs: RECENT: Getting refs >= 2016-01-27 12:08:04.156600185 -0700 MST
trace git-lfs: RECENT: master (2016-02-03 12:08:04 -0700 MST)
trace git-lfs: RECENT: included_branch_2 (2016-01-31 12:08:04 -0700 MST)
trace git-lfs: RECENT: included_branch (2016-01-28 12:08:04 -0700 MST)
--- FAIL: TestRecentBranches (0.56s)
    assert.go:16: /Users/rick/go/src/github.com/github/git-lfs/git/git_test.go:149
    assert.go:25: ! []*git.Ref{
            &git.Ref{Name:"master", Type:0, Sha:"12:08:04.394597 git.c:348               trace: built-in: git 'show' '-s' '--format=%H"},
            &git.Ref{Name:"included_branch_2", Type:0, Sha:"12:08:04.359162 git.c:348               trace: built-in: git 'show' '-s' '--format=%H"},
            &git.Ref{Name:"included_branch", Type:0, Sha:"12:08:04.323946 git.c:348               trace: built-in: git 'show' '-s' '--format=%H"},
        } != []*git.Ref{
            &git.Ref{Name:"master", Type:0, Sha:"c767f84c80519f760b7e267fcaaf8fb3645b6b34"},
            &git.Ref{Name:"included_branch_2", Type:0, Sha:"e608c9454073e3cda9d58cc68bb23afa5ecd8fa0"},
            &git.Ref{Name:"included_branch", Type:0, Sha:"7a2005c9affcd46807f3f32f323fe5417f994360"},
        }
    assert.go:28: !  - Refs should be correct
trace git-lfs: run_command: 'git' rev-parse HEAD --symbolic-full-name HEAD
trace git-lfs: run_command: 'git' version
trace git-lfs: run_command: 'git' rev-parse refs/heads/branch2 --symbolic-full-name refs/heads/branch2
trace git-lfs: run_command: 'git' rev-parse refs/heads/branch4 --symbolic-full-name refs/heads/branch4
trace git-lfs: run_command: 'git' rev-parse refs/heads/master --symbolic-full-name refs/heads/master
--- FAIL: TestWorkTrees (0.36s)
    assert.go:16: /Users/rick/go/src/github.com/github/git-lfs/git/git_test.go:259
    assert.go:25: ! []*git.Ref{
            &git.Ref{Name:"branch2", Type:0, Sha:"12:08:04.831492 git.c:348               trace: built-in: git 'show' '-s' '--format=%H"},
            &git.Ref{Name:"branch4", Type:0, Sha:"12:08:04.946992 git.c:348               trace: built-in: git 'show' '-s' '--format=%H"},
            &git.Ref{Name:"master", Type:0, Sha:"12:08:04.796134 git.c:348               trace: built-in: git 'show' '-s' '--format=%H"},
        } != []*git.Ref{
            &git.Ref{Name:"branch2", Type:0, Sha:"49a0848a1391a4d0dec5cf886386849928281484"},
            &git.Ref{Name:"branch4", Type:0, Sha:"686de8859581179d3d429bb4810da38b51e79bac"},
            &git.Ref{Name:"master", Type:0, Sha:"b7c05066c76446f4dc84f1842d3dadbcc40ff060"},
        }
    assert.go:28: !  - Refs should be correct
FAIL
FAIL    github.com/github/git-lfs/git   1.196s

@technoweenie technoweenie mentioned this pull request Feb 3, 2016
Merged
technoweenie added a commit that referenced this pull request Feb 4, 2016
@technoweenie
Merge pull request #971 from github/skip-trace
5677342 
don't pass GIT_TRACE to exec.Command calls in the git package
@technoweenie technoweenie merged commit 5677342 into master Feb 4, 2016
@technoweenie technoweenie deleted the skip-trace branch February 4, 2016 15:43
@technoweenie technoweenie mentioned this pull request Feb 4, 2016
Closed
15 tasks
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Dec 26, 2022
@chrisd8088
Makefile: unset GIT_TRACE for git commands
0a1ee7b 
In commit c4874df of PR git-lfs#971 the
script/cibuild script was revised to run the Go test suite just for our
"git" Go package in order to confirm that we do not leak the GIT_TRACE
environment variable to Git itself when it is set.

Later, in commit 2235198 of PR git-lfs#3144,
our Makefile was updated to start by running some Git commands
("git describe" and "git rev-parse") to determine the current tagged
version of the project.

When GIT_TRACE=1 is set in second run of the Go test suite, these initial
Git command output Git trace log lines, which does not affect the
validity of the second Go test of our "git" package but does add some
noise to the output of the script/cibuild script.  We therefore just
unset the GIT_TRACE environment variable for these two Git commands in
our Makefile.
@chrisd8088 chrisd8088 mentioned this pull request Dec 26, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
1 participant
@technoweenie