Skip to content

Change git -C reset to git reset#943

Merged
dqminh merged 1 commit intoopencontainers:masterfrom
johnbieren:master
Jul 26, 2016
Merged

Change git -C reset to git reset#943
dqminh merged 1 commit intoopencontainers:masterfrom
johnbieren:master

Conversation

@johnbieren
Copy link
Copy Markdown
Contributor

git -C reset --hard is not supported with older versions of git. For tests/integration/spec.bats I changed from git -C to cd to the directory, run the git reset, then cd back where you were. Very simple change, but it fixes problems with older versions of git.

@wking
Copy link
Copy Markdown
Contributor

wking commented Jul 14, 2016

On Thu, Jul 14, 2016 at 09:08:09AM -0700, Johnny Bieren wrote:

git -C reset --hard is not supported with older
versions of git.

Oops, sorry I suggested it 1 without checking :p. Instead of cd'ing
in and out, you can use a subshell:

(
cd src/runtime-spec &&
run git reset --hard "${SPEC_COMMIT}"
)
[ "$status" -eq 0 ]

which keeps a check for successfully resetting and avoids needing to
‘cd ../..’ and check that for success.

@haiyanmeng
Copy link
Copy Markdown
Contributor

Here is some details about the problem:
The git version on RHEL7/centos7 is 1.8.3.1, which does not support -C option. Currently, runc/tests/integration/spec.bats uses the -C option of git.

@mrunalp
Copy link
Copy Markdown
Contributor

mrunalp commented Jul 18, 2016

@johnbieren Could you update the PR with @wking suggestion?

@johnbieren
Copy link
Copy Markdown
Contributor Author

@mrunalp done

@wking
Copy link
Copy Markdown
Contributor

wking commented Jul 25, 2016 via email

@johnbieren
Copy link
Copy Markdown
Contributor Author

@wking I have fixed the indentation and can confirm that the test passes as written and putting a rm -rf src/runtime-spec in front of the subshell forces the test to fail

@wking
Copy link
Copy Markdown
Contributor

wking commented Jul 25, 2016

On Mon, Jul 25, 2016 at 12:07:23PM -0700, Johnny Bieren wrote:

@wking I have fixed the indentation and can confirm that the test
passes as written and putting a rm -rf src/runtime-spec in front of
the subshell forces the test to fail

Thanks for testing :). 6c7d2bb looks good to me, although I'd have
indented the contents of the subshell further:

  • run git -C src/runtime-spec reset --hard "${SPEC_COMMIT}"
  • (
  • cd src/runtime-spec &&
  • run git reset --hard "${SPEC_COMMIT}"
  • )

Comment thread tests/integration/spec.bats Outdated
SPEC_COMMIT=$(grep runtime-spec ${TESTDIR}/../../Godeps/Godeps.json -A 4 | grep Rev | cut -d":" -f 2 | tr -d ' "')
run git -C src/runtime-spec reset --hard "${SPEC_COMMIT}"
(
cd src/runtime-spec &&
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, let's indent this more.

Signed-off-by: Johnny Bieren <jbieren@redhat.com>
@mrunalp
Copy link
Copy Markdown
Contributor

mrunalp commented Jul 26, 2016

LGTM

Approved with PullApprove

1 similar comment
@dqminh
Copy link
Copy Markdown
Contributor

dqminh commented Jul 26, 2016

LGTM

Approved with PullApprove

@dqminh dqminh merged commit 5226749 into opencontainers:master Jul 26, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants