Skip to content
This repository was archived by the owner on Jan 20, 2025. It is now read-only.

Fix clean up of downloaded modules when using go.mod #50

Merged

Conversation

electrofelix
Copy link
Contributor

Projects that use go.mod will result in the build downloading the
modules to an temporary path. Golang introduced read only directories to
protect the generated module cache from accidental writes, resulting in
failures to build when exiting the temporary directory context.

Adapt the solution from pre-commit to make the directory and file
writable on removal and include a rudimentary test that exercises
download via a simple example module hosted by the golang org in github.

Fixes #49

@electrofelix
Copy link
Contributor Author

Interestingly, based on the coverage report it appears that 1.12 left the module cache directories writable as it appears the onerror never triggers for that version. Any suggestions on what to do here?

@asottile
Copy link
Member

probably what I suggested here: #49 (comment)

@electrofelix
Copy link
Contributor Author

@asottile I updated the code to use that mechanism, the problem is that for golang 1.12 it appears the onerror handler is never triggered resulting in a less than 100% code coverage https://asottile.visualstudio.com/asottile/_build/results?buildId=4453&view=logs&j=78aa2813-093d-53ab-16fb-7109f694999b&t=7a1af5e5-0a3d-5102-fed3-6e52dddd3fe5&l=35

I'm not sure how I can get 100% coverage here with the run against golang 1.12 and golang 1.13 having coverage assessed separately

@asottile
Copy link
Member

oh sorry it wasn't clear:

can probably use the same trick that pre-commit uses if you'd like to send a patch and bump the tested go versions to the latest two (those are all that upstream supports)

@electrofelix
Copy link
Contributor Author

@asottile ah, you meant bump to 1.14 and 1.15

@electrofelix
Copy link
Contributor Author

@asottile should https://github.com/asottile/setuptools-golang/blob/master/setuptools_golang.py#L191 be bumped as well to default to a tested version? and if so to 1.15.1 or 1.14.8?

@asottile
Copy link
Member

@asottile should https://github.com/asottile/setuptools-golang/blob/master/setuptools_golang.py#L191 be bumped as well to default to a tested version? and if so to 1.15.1 or 1.14.8?

yeah I usually bump all 3 at the same time (do you want to separate that patch out to a separate PR?)

@electrofelix
Copy link
Contributor Author

sure, I'll put one up in a while, might be tomorrow though

@asottile
Copy link
Member

sounds good -- no problem! thanks for working on this :)

@electrofelix
Copy link
Contributor Author

@asottile meant to ask, do you prefer the PR to have a clean history e.g. rebase onto latest, or squash on merge? Just that this has obviously picked up a few additional merges.

@asottile
Copy link
Member

rebase would be nice if you're willing

@electrofelix electrofelix force-pushed the support-gomodule-download branch from 16f7c4c to a8c7e50 Compare September 23, 2020 15:26
Projects that use go.mod will result in the build downloading the
modules to an temporary path. Golang introduced read only directories to
protect the generated module cache from accidental writes, resulting in
failures to build when exiting the temporary directory context.

Adapt the solution from pre-commit to make the directory and file
writable on removal and include a rudimentary test that exercises
download via a simple example module hosted by the golang org in github.

Fixes asottile-archive#49
@electrofelix electrofelix force-pushed the support-gomodule-download branch from a8c7e50 to 0a61e39 Compare September 23, 2020 15:27
@electrofelix
Copy link
Contributor Author

Done, corrected the function description on rmtree to indicate it's no longer limited to Windows only and instead required due to newer versions of golang making module caches read-only.

Copy link
Member

@asottile asottile left a comment

Choose a reason for hiding this comment

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

@asottile asottile merged commit bc56a8c into asottile-archive:master Sep 23, 2020
@asottile
Copy link
Member

thanks again! I've uploaded 2.2.0 with this and the other changes \o/

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use of go.mod fails with golang 1.13+ unless run as root
2 participants