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

Remove explicit LISTEN_PORT from MSI installer #883

Merged
merged 1 commit into from
Dec 18, 2021
Merged

Remove explicit LISTEN_PORT from MSI installer #883

merged 1 commit into from
Dec 18, 2021

Conversation

breed808
Copy link
Contributor

@breed808 breed808 commented Dec 2, 2021 •
edited
Loading

Explicit setting of listening port in the service definition causes port
setting in configuration file to be ignored.

Exporter already defines a default port (9182) if one is not specified,
so no impact from this change is anticipated.

Resolves #881

@breed808 breed808 requested a review from a team as a code owner December 2, 2021 01:57
carlpett
carlpett approved these changes Dec 4, 2021
View reviewed changes
Copy link
Collaborator

@carlpett carlpett left a comment

Choose a reason for hiding this comment

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

👍 Like the approach! Much simpler than what I saw a one-liner would be, nice idea of splitting them up instead!

@breed808
Copy link
Contributor Author

breed808 commented Dec 5, 2021 •
edited
Loading

I've just managed to test MSI creation with this branch, and WIX is not happy with the duplicate symbols:

PS C:\Users\Ben\Desktop\windows_exporter> .\installer\build.ps1 -PathToExecutable .\windows_exporter.exe -Version 0.16.0 -Arch "amd64"
windows_exporter.wxs
C:\Users\Ben\Desktop\windows_exporter\installer\windows_exporter.wxs(27) : error LGHT0091 : Duplicate symbol 'CustomAction:SetListenFlag' found. This typically means that an Id is duplicated. Check to make sure all your identifiers of a given type (File, Component, Feature) are unique.                                                                          C:\Users\Ben\Desktop\windows_exporter\installer\windows_exporter.wxs(28) : error LGHT0092 : Location of symbol related to previous error.                                           C:\Users\Ben\Desktop\windows_exporter\installer\windows_exporter.wxs(27) : error LGHT0091 : Duplicate symbol 'CustomAction:SetListenFlag' found. This typically means that an Id is duplicated. Check to make sure all your identifiers of a given type (File, Component, Feature) are unique.                                                                          C:\Users\Ben\Desktop\windows_exporter\installer\windows_exporter.wxs(29) : error LGHT0092 : Location of symbol related to previous error.
C:\Users\Ben\Desktop\windows_exporter\installer\windows_exporter.wxs(27) : error LGHT0091 : Duplicate symbol 'WixAction:InstallExecuteSequence/SetListenFlag' found. This typically means that an Id is duplicated. Check to make sure all your identifiers of a given type (File, Component, Feature) are unique.
C:\Users\Ben\Desktop\windows_exporter\installer\windows_exporter.wxs(28) : error LGHT0092 : Location of symbol related to previous error.
C:\Users\Ben\Desktop\windows_exporter\installer\windows_exporter.wxs(27) : error LGHT0091 : Duplicate symbol 'WixAction:InstallExecuteSequence/SetListenFlag' found. This typically means that an Id is duplicated. Check to make sure all your identifiers of a given type (File, Component, Feature) are unique.
C:\Users\Ben\Desktop\windows_exporter\installer\windows_exporter.wxs(29) : error LGHT0092 : Location of symbol related to previous error.

I'll have to find a way to implement this on a single line, which won't be pretty.

@breed808
Remove explicit LISTEN_PORT from MSI installer
45e9357 
Explicit setting of listening port in the service definition causes port
setting in configuration file to be ignored.

Exporter already defines a default port (9812) if one is not specified,
so no impact from this change is anticipated.

Signed-off-by: Ben Reedy <breed808@breed808.com>
@breed808
Copy link
Contributor Author

I believe I have it now. Using distinct Wix Id values worked, but it's no longer a one-liner.

carlpett
carlpett approved these changes Dec 18, 2021
View reviewed changes
Copy link
Collaborator

@carlpett carlpett left a comment

Choose a reason for hiding this comment

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

I suppose you could set a new property ListenFlag to the listing of all three if you want to shorten the ServiceInstall line, but I'm okay with this :)
Nice work!

@breed808 breed808 merged commit c7d07a3 into prometheus-community:master Dec 18, 2021
@breed808 breed808 deleted the msi_listen_port branch December 18, 2021 22:30
anubhavg-icpl pushed a commit to anubhavg-icpl/windows_exporter that referenced this pull request Sep 22, 2024
@breed808
Merge pull request prometheus-community#883 from breed808/msi_listen_…
b5289ac 
…port

Remove explicit LISTEN_PORT from MSI installer
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@carlpett carlpett carlpett approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

telemetry: addr: not work in config file. (0.16.0)
2 participants
@breed808 @carlpett