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

Windows: Build tetragon on Windows #3440

Closed

Conversation

ExceptionalHandler
Copy link

Description

This change attempts to build tetragon on Windows. Following is the strategy

1. Code that is used in both platform, but is different, is separated by _linux.go and _windows.go suffixes

2. Code that is unique to each OS is compiled with approrpiate //go:build directive

3. certain constants are moved to a separate constants package

Caveats:
This is a compilation change. Tetragon.exe on windows runs and shows help message but does not monitor any activity.

Changelog

Tetragon.exe can now be compiled on Windows

Copy link
Member

@mtardy mtardy left a comment

Choose a reason for hiding this comment

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

Hey Anadi thanks for that massive amount of work! 🪟

What would you thinkk about maybe splitting this into different commits for example:

  • commits for build directive/renaming _linux _windows
  • many multiple commits for logical changes that were needed in the codebase

@mtardy mtardy added the release-note/major This PR introduces major new functionality label Feb 26, 2025
@ExceptionalHandler
Copy link
Author

@mtardy , That was the original plan, but the logical changes turned out to be minimal and restricted to only seven files. We needed those to build on Windows. Most changes are addition of //go:build directive

@olsajiri
Copy link
Contributor

heya,
looks great! quick comments before I dive in (might take a while):

  • could we keep the linux end of lines, some files seem to have 0x0d 0x0a chars
  • there's tetragon binary pushed in and removed, let's not add it in the first place
  • second patch seems to be unrelated/separated fix, please add changelog to it

thanks

@olsajiri
Copy link
Contributor

but the logical changes turned out to be minimal and restricted to only seven files. We needed those to build on Windows. Most changes are addition

+1 @mtardy .. I think it should be still possible to move the changes into several separate parts, does not need to be commit per package, but bigger some bigger commits granularity would ease the review

Copy link

netlify bot commented Feb 27, 2025

Deploy Preview for tetragon ready!

Name Link
🔨 Latest commit 6e68e37
🔍 Latest deploy log https://app.netlify.com/sites/tetragon/deploys/67c0dd35794eed0008b683d3
😎 Deploy Preview https://deploy-preview-3440--tetragon.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

…trategy

1. Code that is used in both platform, but is different, is separated by _linux.go and _windows.go suffixes.
2. Some packages now therefore have three go files seperating common, linux and windows code
3. the _windows.go files will be in a separate PR
4. Code that is unique to each OS is compiled with approrpiate //go:build directive
5. Certain constants are moved to a separate constants package

Signed-off-by: Anadi Anadi <aanadi@cisco.com>
@ExceptionalHandler
Copy link
Author

Closing this PR.
New PR on the way with removed _windows.go files and synced again main

@mtardy
Copy link
Member

mtardy commented Feb 28, 2025

Closing this PR. New PR on the way with removed _windows.go files and synced again main

hey, you could rebase so that we have the history of the discussion in the same PR. You can use --force-with-lease to force push to your own branches :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/major This PR introduces major new functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants