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.

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

Check if SHA1 exists before calling git diffon it #42

Merged
merged 1 commit into from Oct 19, 2016

Conversation

asymmetric
Copy link
Contributor

In the case of a force push, the commit will not exist anymore, and the
git diff in has_changes will fail, but the failure would be silently
ignored (since the error message would be passed to wc).

wc -l would return 1, so effectively the same image would be reused
over and over.

@asymmetric
Copy link
Contributor Author

@tomwilkie Here's the change from weaveworks/scope#1924.

Not sure why the CI build is failing though.

@@ -40,13 +40,27 @@ commit_timestamp() {
git show -s --format=%ct "$rev"
}

# is the SHA1 actually present in the repo?
# it cold be it isn't, e.g. after a force push

This comment was marked as abuse.

commit_valid() {
local rev=$1
git rev-parse --quiet --verify "$rev^{commit}" > /dev/null
[ $? -eq 0 ]

This comment was marked as abuse.

In the case of a force push, the commit might not exist anymore, and the
`git diff` in `has_changes` will fail, but the failure would be silently
ignored (since the error message would be passed to `wc`).

`wc -l` would return 1, so effectively the same image would be reused
over and over.
@asymmetric
Copy link
Contributor Author

@2opremio updated, could you please take a look?

@2opremio
Copy link
Contributor

The CircleCI build is breaking with golang/lint#246 . Can you try running a more recent version of Go?

@2opremio
Copy link
Contributor

I tried changing the Version run by CircleCI to Ubuntu 14.04 (Trusty) (which requires a new commit to take effect), but it won't probably be enough.

@2opremio
Copy link
Contributor

LGTM, let's fix the build issue separately

@2opremio 2opremio merged commit b990f48 into weaveworks:master Oct 19, 2016
@asymmetric asymmetric deleted the lorenzo/fix-git-diff branch October 19, 2016 10:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants