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

Product/File version in Windows executables #3414

Closed
pocki opened this issue Nov 29, 2021 · 18 comments · Fixed by #4811
Closed

Product/File version in Windows executables #3414

pocki opened this issue Nov 29, 2021 · 18 comments · Fixed by #4811
Labels
good first issue Good for beginners help wanted Features that maintainers are willing to accept but do not have cycles to implement

Comments

@pocki
Copy link

pocki commented Nov 29, 2021

Requirement - what kind of business use case are you trying to solve?

Actual the windows executables (jaeger-agent.exe, jaeger-collector.exe, ...) don't have a version set. It is hard to identify the current used version and check if the actual used version is outdated.

image

Problem - what in Jaeger blocks you from solving the requirement?

Checking the actual used version and identity an outdated version.
We have some applications running native on Windows (Server) and therefore the agent is also running native on Windows.

Proposal - what do you suggest to solve the problem or improve the existing situation?

Set the Product and/or File Version of the windows executables on compile or release.

Any open questions to address

@yurishkuro
Copy link
Member

Jaeger binaries support "version" subcommand, is that not enough?

@pocki
Copy link
Author

pocki commented Nov 29, 2021

Thanks for the information. It's a little bit hidden.
Normally the services are always running and you just want to check the version.
On Windows users are more likely to check it the properties page instead of extra typing in a command and starting the subcommand (also when the parameter for version nearly every app uses different)

As addition to the subcommand the product/file version would be very nice to have

@yurishkuro
Copy link
Member

I don't know if Go compiler supports setting these properties on the Windows binary, but I don't have any objections to including them.

@yurishkuro yurishkuro added help wanted Features that maintainers are willing to accept but do not have cycles to implement good first issue Good for beginners labels Nov 29, 2021
@pocki
Copy link
Author

pocki commented Nov 29, 2021

Sorry, I am not familiar with Go and Go compiler.

@grammicus
Copy link

Hi can I take this?

@yurishkuro
Copy link
Member

@smolgeat sure

@xmarcoied
Copy link

xmarcoied commented Apr 19, 2022

Hi @yurishkuro.

I don't know if Go compiler supports setting these properties on the Windows binary, but I don't have any objections to including them.

I don't think there's a straightforward way to apply such configurations with Go Compile. but I am thinking to use a 3rd party service/repo to embed such info through a go:generate. What do you think about this approach? I can draft a PR for one.

@yurishkuro
Copy link
Member

That might be ok, I would recommend checking what other projects do (e.g. does Hugo have Windows binaries? Do they tag them like that?)

@keremgocen
Copy link

looking for a beginner-friendly issue if this is available

@yurishkuro
Copy link
Member

it is

@ananyak19
Copy link

Hey can I work on this?

@yurishkuro
Copy link
Member

Yes

@ananyak19
Copy link

/assign

@ananyak19
Copy link

Hi @pocki I am a first time contributor, can you please guide on how to get started here and where I need to make the changes in order to solve the issue?

@pocki
Copy link
Author

pocki commented Oct 4, 2022

As you can see above, I am also not familiar with Go and the Go Compiler.
Please check the contribution guidelines for this project.

@yurishkuro
Copy link
Member

I see references to two utilities that seem to achieve this: https://stackoverflow.com/questions/35126344/how-to-set-the-file-version-field-in-pe-header-of-exe-files

@ananyak19
Copy link

Hi, @pocki @yurishkuro i tried to clone the repository but don't have the required access rights. Can you help me with that?

@yurishkuro
Copy link
Member

you need to clone into your personal namespace, e.g. ananyak19/jaeger

yurishkuro pushed a commit that referenced this issue Oct 8, 2023
## Which problem is this PR solving?
This resolves #3414

## Description of the changes
Add to the windows build step in `Makefile` to create a `resource.syso`
using [goversioninfo](https://github.com/josephspurrier/goversioninfo).
The version is extracted from the latest git tag.

`go build` will embed this information in the final executable


![image](https://github.com/jaegertracing/jaeger/assets/6261556/5c45219c-4528-4000-b5af-cd0fd1e2a421)

(Forgive the German in this screenshot)

## How was this change tested?
1. Run ` make build-binaries-windows` 
2. Navigate to the folder containing binaries (e.g. `
cmd/agent/agent-windows-amd64`)
3. Add `.exe` to the file name for the changes to be seen:
`agent-windows-amd64.exe`

Ran other targets (`make build-binaries-linux`) to see if it broke other
targets. Looked fine so far.


## Notes
There is the hassle of creating a `.syso` file that must be in the [same
directory as the main.go
file](golang/go#16090 (comment))
so there's a lot of copying to each `cmd/*`.

I tried to keep it centralized to a `prepare-winres` Makefile target and
the files are cleaned up afterwards.

## Checklist
- [x] I have read
https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
- [x] I have signed all commits
- [ ] I have added unit tests for the new functionality
- [x] I have run lint and test steps successfully
  - for `jaeger`: `make lint test`
  - for `jaeger-ui`: `yarn lint` and `yarn test`

Signed-off-by: Julien Midedji <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for beginners help wanted Features that maintainers are willing to accept but do not have cycles to implement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants