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

refactor(compile): add trimpath to compile to remove local paths #457

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

sheldonhull
Copy link

  • Include trimpath to ensure local filepaths from CI or developer don't get included in the produced artifact.
  • This was added in Go 1.13, so not sure if there is something special to handle adding this flag only if go version > 1.13.

Go documentation on this:

-trimpath
remove all file system paths from the resulting executable.
Instead of absolute file system paths, the recorded file names
will begin either a module path@version (when using modules),
or a plain import path (when using the standard library, or GOPATH).

- Include trimpath to ensure local filepaths from CI or developer don't get included in the produced artifact.
- This was added in Go 1.13, so not sure if there is something special to handle adding this flag only if go version > 1.13.
@jaredallard
Copy link
Contributor

jaredallard commented Feb 18, 2023

I feel like the aqua part should be called out in the title/desc somewhere....

EDIT: Ah, I see, that is from #451 and just wasn't removed when making this PR. I'd suggest cleaning that up :D

@sheldonhull
Copy link
Author

sheldonhull commented Feb 19, 2023 via email

@sheldonhull
Copy link
Author

@jaredallard reverted back so pr changes are now just this. Not sure about compatibility for prior go versions to 1.13 like I mentioned but otherwise this will be good for privacy. Right now compiled binary includes full path. Cheers

mage/main.go Outdated Show resolved Hide resolved
Copy link
Contributor

@jaredallard jaredallard left a comment

Choose a reason for hiding this comment

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

Looks reasonable to me :D

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.

3 participants