Skip to content

remove legacy build-tags #5904

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

Merged
thaJeztah merged 2 commits into from
Mar 10, 2025
Merged

remove legacy build-tags #5904

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
46caf56
remove legacy build-tags
thaJeztah Mar 8, 2025
f6d49e9
docs: remove legacy build-tags
thaJeztah Mar 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion cli/command/container/completion.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
// FIXME(thaJeztah): remove once we are a module; the go:build directive prevents go from downgrading language version to go1.16:
//go:build go1.22
// +build go1.22

package container

Expand Down
3 changes: 1 addition & 2 deletions cmd/docker/internal/signals/signals_unix.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
//go:build unix
// +build unix
//go:build !windows

@Benehiko Benehiko Mar 10, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

!windows is a lot broader than just unix. The package used here is golang.org/x/sys/unix which I assume supports all unix systems, but it's unclear to me if !windows would be too broad for it, wdyt?

@vvoland vvoland Mar 10, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

In this case we mostly care about complementing the _windows file, so it's fine.

If someone attempts to build the CLI for such platform, I don't think it makes much difference if they would get an TerminationSignals is undefined vs package unix doesn't provide SIGTERM/SIGINT.

@thaJeztah thaJeztah Mar 10, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I originally removed the build-tag, but forgot that unix is "special" in Go, because for backward-compatibility, they didn't add support for _unix.go as suffix (as they do for other build-tags, like _windows.go.

I can add a signals_other.go or signals_unsupported.go instead that defines an empty slice instead?

var TerminationSignals = []os.Signal{}

or

var TerminationSignals []os.Signal

@Benehiko let me know what you prefer 👍

@Benehiko Benehiko Mar 10, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it's fine as is :)

@thaJeztah thaJeztah Mar 10, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Ah, I also just realised that this is an internal package inside cmd/docker, so really only for building the binary, and not to be used by anyone else, so probably should be fine yes.


package signals

Expand Down
1 change: 0 additions & 1 deletion docs/generate/generate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// This file is intended for use with "go run"; it isn't really part of the package.

//go:build docsgen
// +build docsgen

package main

Expand Down
1 change: 0 additions & 1 deletion docs/generate/tools.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
//go:build tools
// +build tools

package main

Expand Down