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

chore: Using Windows, add exe extension to tools #11608

Merged
merged 6 commits into from
Aug 3, 2022
Merged

Conversation

sspaink
Copy link
Contributor

@sspaink sspaink commented Aug 3, 2022

When building Telegraf on a Windows machine, the new tools were being built without exe extension causing the following error:

plugins\inputs\wireguard\wireguard.go:1: running "../../../tools/readme_config_includer/generator": exec: "H:\\Sandbox\\telegraf\\tools\\readme_config_includer\\generator": file does not exist for each plugin

Updated the makefile to add exe extension if GOOS is set to windows.

@telegraf-tiger telegraf-tiger bot added the chore label Aug 3, 2022
Copy link
Contributor

@powersj powersj left a comment

Choose a reason for hiding this comment

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

Thank you! This will fix 1 of the 3 issues in #11609

Makefile Outdated Show resolved Hide resolved
@barts2108
Copy link

Aren't the tools/licence_checker/licence_checker and tools/readme_config_includer/generator supposed to get the .exe extension on windows ? Both the licence_checker (without extension) and generator (without extension) are binary files. If I modify the makefile to use this :

build_tools:
 $(HOSTGO) build -o ./tools/license_checker/license_checker$(EXEEXT) ./tools/license_checker
 $(HOSTGO) build -o ./tools/readme_config_includer/generator$(EXEEXT) ./tools/readme_config_includer/generator.go

with the makefile change mentioned above, and temporary changing HOSTGO to HOSTGO = go
I can build telegraf.exe from a simple cmd.exe and no MingW requirement. However that is building for WIndows on WIndows

@sspaink
Copy link
Contributor Author

sspaink commented Aug 3, 2022

@barts2108 your right the tools do need the .exe extension for windows which is what this pull request will fix. For fixing HOSTGO, I was able to get it working by changing it to HOSTGO := set $(GOOS)= & set $(GOARCH)= & go. The only issue I don't know how to fix is running make clean, which would require using if exists filename del /f filename or if exists folder rmdir /s /q folder but then we need someway to detect what terminal is being used because this would break using bash on windows but I am unable to find anything reliable.

Is using cmd.exe or powershell a requirement for you to develop Telegraf? Otherwise at this point I would rather just update the documentation to say we require MinGW to avoid making the makefile anymore confusing.

@powersj
Copy link
Contributor

powersj commented Aug 3, 2022

HOSTGO := set $(GOOS)= & set $(GOARCH)= & go

nice find!

I don't know how to fix is running make clean

Does power shell have del? We could do del telegraf >nul 2>&1

@barts2108
Copy link

Did you notice i added the EXEEXT in those two lines in the build_tools?

cmd or powershe is not a real requirement. However when you just add a requirement for MingW, then please add some documentation too how it must be setup and how it is used. Currently the readme that i used only containse a git clone, a cd and a make instruction. Nothing about environment variables or so.

@barts2108
Copy link

barts2108 commented Aug 3, 2022

HOSTGO := set $(GOOS)= & set $(GOARCH)= & go

nice find!

I don't know how to fix is running make clean

Does power shell have del? We could do del telegraf >nul 2>&1

'rmdir /s /q telegraf'

Edit: i think i forgot the need for some universal commands. But why not using if statements for commands that are different on windows ?

Makefile Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
@telegraf-tiger
Copy link
Contributor

telegraf-tiger bot commented Aug 3, 2022

@sspaink sspaink merged commit fa811ed into master Aug 3, 2022
@sspaink sspaink deleted the fixwindowsdev branch August 3, 2022 22:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants