Skip to content
This repository has been archived by the owner on Feb 1, 2024. It is now read-only.

[1](2) add GOARM versions in metrics, or remove TODO and explain in comment why its missing #567

Closed
nikhilsaraf opened this issue Oct 26, 2020 · 4 comments · Fixed by #579
Assignees
Labels
bug Something isn't working
Milestone

Comments

@nikhilsaraf
Copy link
Contributor

No description provided.

@nikhilsaraf nikhilsaraf added the bug Something isn't working label Oct 26, 2020
@nikhilsaraf nikhilsaraf added this to the v1.10.1 milestone Oct 26, 2020
@debnil
Copy link
Contributor

debnil commented Oct 29, 2020

Currently, this is missing because the runtime package doesn't expose GOARM, and it's not clear there is a standard way to do this using Go packages.

We can potentially do this through defining and exporting a GOARM variable from the build script. Should we follow this path?

@nikhilsaraf
Copy link
Contributor Author

yes, build script makes sense. we will need to be careful since this will be the first case of an ldflags parameter that is not global for the entire build script and needs to be injected correctly for each and every build. not sure if you can have multiple -ldflags cli options when building but if you can then that's perfect. if not we will need to maybe do some string manipulation to get it working correctly

@debnil
Copy link
Contributor

debnil commented Nov 6, 2020

I'm not sure how we inject this correctly, and I don't think you can have multiple -ldflags options on build.

But, even if we can do those things, I'm not sure we can reliably define GOARM for users not using the hand-rolled binaries. Currently, GOARM is only defined on deploy (here), and it uses specific choices of GOARM that are defined in PLATFORM_ARGS (here). It isn't done earlier in the script when the cmd package variables are exported (here). So, I don't know if this export is possible for users compiling from source. If it's not, I'd lean towards just not defining GOARM for those users, which substantially simplifies the overall logic. Thoughts?

@nikhilsaraf
Copy link
Contributor Author

@debnil can we do something where we create a temporary LDFLAGS_WITH_GOARM variable that is "LDFLAGS + GOARM" (here)?

nikhilsaraf pushed a commit that referenced this issue Nov 12, 2020
* Initial commit

* Remove unrelated UI changes.

* Fix typo
@debnil debnil changed the title [1] add GOARM versions in metrics, or remove TODO and explain in comment why its missing [1](2) add GOARM versions in metrics, or remove TODO and explain in comment why its missing Nov 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants