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

Upload single sha256sums file for dist build #266

Merged
merged 3 commits into from Oct 21, 2019
Merged

Conversation

csstaub
Copy link
Member

@csstaub csstaub commented Oct 18, 2019

Better than having lots of small files for each binary, makes the release a bit cleaner. I did this for the v1.5.1 release to patch the recent Go CVE, this PR is the cherry-pick on master.

@coveralls
Copy link

coveralls commented Oct 18, 2019

Coverage Status

Coverage remained the same at 89.563% when pulling e7e7a30 on cs/combined-sha256sums into 7e3bf78 on master.

Copy link
Collaborator

@mbyczkowski mbyczkowski left a comment

Choose a reason for hiding this comment

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

question around collecting the SHA sums in one file, but other than that good idea.

@@ -61,4 +59,5 @@ dist:
@$(MAKE) -f Makefile ghostunnel.man
@mv ghostunnel.man "${CURRENT_DIR}/dist/ghostunnel-${VERSION}.man"
@$(MAKE) -f "${MKFILE_PATH}" -j1 build
@cd "${CURRENT_DIR}/dist/" && sha256sum ghostunnel-* > sha256sums.txt
Copy link
Collaborator

Choose a reason for hiding this comment

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

would this actually append instead of override?

Copy link
Member Author

Choose a reason for hiding this comment

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

It should overwrite, since I used > (as opposed to >>).

Copy link
Member Author

Choose a reason for hiding this comment

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

Also, to be clear, we want to overwrite here. At this point in the script all binaries have been created and we sha256sum them all at once.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Good context, yes this is exactly what I was wondering here.

@csstaub csstaub merged commit 6631578 into master Oct 21, 2019
@csstaub csstaub deleted the cs/combined-sha256sums branch October 21, 2019 02: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

3 participants