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

Makefile sha1sum is not available on all OSs #3630

Closed
4 tasks done
greg-szabo opened this issue Feb 12, 2019 · 11 comments
Closed
4 tasks done

Makefile sha1sum is not available on all OSs #3630

greg-szabo opened this issue Feb 12, 2019 · 11 comments
Assignees
@alessio
Milestone
Backlog

Comments

@greg-szabo
Copy link
Member

greg-szabo commented Feb 12, 2019 •
edited by alexanderbez
Loading

Summary of Bug

The vendor-deps target in the Makefile uses sha1sum for some checks. It is not available on Macs and Windows. Makefile has to be cross-OS compatible. Please fix or remove.

Note: apparently sha1sum is supposed to hash the whole vendor folder. Depending on FS implementations this could be different on each OS. My advice: this should be removed.

Steps to Reproduce

make vendor-deps on OSX or Windows.

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@alexanderbez
Copy link
Contributor

Agree here @greg-szabo -- do you have any recommendations?

@jackzampolin
Copy link
Member

jackzampolin commented Feb 12, 2019 •
edited
Loading

I did write gosum for this exact usecase https://github.com/jackzampolin/gosum

We could go get it during the tools step...

@alexanderbez
Copy link
Contributor

@alessio bump

@alexanderbez alexanderbez mentioned this issue Mar 11, 2019
Closed
@greg-szabo
Copy link
Member Author

My advice is the ticket description: remove this feature for now.

Dependency management will change with go1.13 with Go modules. By that time, we can describe requirements for this feature better.

@alexanderbez
Copy link
Contributor

Indeed the modules file will contain a hash which is what we need. I suppose we can get rid of vendor hash all together @alessio

@alessio
Copy link
Contributor

alessio commented Mar 12, 2019

Once we switched to go modules, we can get rid of it

alessio added a commit that referenced this issue Mar 15, 2019
@alessio
Replace sha1sum with jack's gosum
5efb069 
Also don't compute hash on vendor/ contents. Instead hash go.sum.

Closes: #3630
@alessio alessio mentioned this issue Mar 15, 2019
Merged
5 tasks
@alexanderbez
Copy link
Contributor

Closing as we have #3907

@alessio
Copy link
Contributor

alessio commented Mar 15, 2019

@alexanderbez I'd keep this open til #3907 is merged

@alessio alessio reopened this Mar 15, 2019
@alexanderbez
Copy link
Contributor

Fair, but we don't need it in this milestone -- especially since #3907 will be merged.

@alexanderbez alexanderbez modified the milestones: v0.34.0, Backlog Mar 15, 2019
@alessio
Copy link
Contributor

alessio commented Mar 15, 2019

agreed

alessio added a commit that referenced this issue Mar 17, 2019
@alessio
replace dep with go mod
5aa88ce 
Replace sha1sum with jack's gosum and get rid of
vendor-deps.
Also don't compute hash on vendor/ contents.
Instead hash go.sum.

Disable unconvert lint check. It does not
work very well with go mod.

Remove update_vendor_deps once and for all.

Closes: #3919 #3630
alessio added a commit that referenced this issue Mar 18, 2019
@alessio
replace dep with go mod (#3907)
6ce4d5e 
Replace sha1sum with jack's gosum and get rid of
vendor-deps.
Also don't compute hash on vendor/ contents.
Instead hash go.sum.

Disable unconvert lint check. It does not
work very well with go mod.

Remove update_vendor_deps once and for all.

Upgrade to go 1.12

Closes: #3919 #3630
@alessio
Copy link
Contributor

alessio commented Mar 18, 2019

Fixed in #3907. Hence closing.

@alessio alessio closed this as completed Mar 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@alessio alessio

Labels
None yet
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
4 participants
@alessio @alexanderbez @jackzampolin @greg-szabo