pkg/reexec: cleanup and remove some dependencies#47932
Merged
thaJeztah merged 5 commits intomoby:masterfrom Jun 10, 2024
Merged
pkg/reexec: cleanup and remove some dependencies#47932thaJeztah merged 5 commits intomoby:masterfrom
thaJeztah merged 5 commits intomoby:masterfrom
Conversation
commit 069fdc8 changed most uses of the syscall package to switch utsname from unsigned to signed (see 069fdc8). Those don't seem to be impacting the code used here, so either stdlib or golang.org/x/sys/unix should work for this case. I chose stdlib's syscall package for this case, in case we'd decide to move this package to a separate module (and want to limit its dependencies). Signed-off-by: Sebastiaan van Stijn <[email protected]>
This combines the implementations of the Self function, to allow having a single GoDoc to document the behavior. The naiveSelf function is kept, because it's used in unit-tests. There is a minor change in behavior, as this patch removes the stub for unsupported platforms (non-linux, windows, freebsd or darwin), which will now use `os.Args[0]`. The stub was added in 21537b8 to fix compilation of https://github.com/ethereum/go-ethereum on OpenBSD, which had docker/docker as dependency. It looks like that repository no longer has this dependency, and as this was only to make the code compilable, is unlikely to be a problem. Signed-off-by: Sebastiaan van Stijn <[email protected]>
The Windows, Darwin, and FreeBSD implementations were identical, other than their GoDoc to be different. Unify them so that we don't have to maintain separate GoDoc for each. It's worth noting that FreeBSD also supports Pdeathsig, so could be using the same implementation as Linux. However, we don't test/maintain the FreeBSD implementation, and it would require updating to GoDoc to be more specific about the use of `/proc/self/exe`, so keeping the status quo for now. Signed-off-by: Sebastiaan van Stijn <[email protected]>
This package is used in BuildKit, and could be a potential candidate for moving to a separate module. While it's not too problematic to have this dependency, the tests only used basic assertions from gotest.tools, which could be easily re-implemented without the dependency. Signed-off-by: Sebastiaan van Stijn <[email protected]>
Touch-up some GoDoc in the package, and remove "import" comments. This package is used in BuildKit, and could be a potential candidate for moving to a separate module. The "import" comments are ignored when used in go module mode so have little benefit. Let's remove them. Signed-off-by: Sebastiaan van Stijn <[email protected]>
thaJeztah
added
status/2-code-review
kind/refactor
vvoland
approved these changes
Jun 10, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
/pkg#32989pkg/reexec: don't mix syscall and golang.org/x/sys package
commit 069fdc8 changed most uses of
the syscall package to switch utsname from unsigned to signed (see
069fdc8). Those don't seem to be
impacting the code used here, so either stdlib or golang.org/x/sys/unix
should work for this case.
I chose stdlib's syscall package for this case, in case we'd decide to
move this package to a separate module (and want to limit its dependencies).
pkg/reexec: unify implementation of Self() and remove stub
This combines the implementations of the Self function, to allow having
a single GoDoc to document the behavior.
There is a minor change in behavior, as this patch removes the stub for
unsupported platforms (non-linux, windows, freebsd or darwin), which will
now use
os.Args[0]. The stub was added in 21537b8to fix compilation of https://github.com/ethereum/go-ethereum on OpenBSD,
which had docker/docker as dependency. It looks like that repository no
longer has this dependency, and as this was only to make the code
compilable, is unlikely to be a problem.
pkg/reexec: unify non-Linux implementation of Command
The Windows, Darwin, and FreeBSD implementations were identical, other
than their GoDoc to be different. Unify them so that we don't have to
maintain separate GoDoc for each.
It's worth noting that FreeBSD also supports Pdeathsig, so could be
using the same implementation as Linux. However, we don't test/maintain
the FreeBSD implementation, and it would require updating to GoDoc to
be more specific about the use of
/proc/self/exe, so keeping thestatus quo for now.
pkg/reexec: remove gotest.tools from tests
This package is used in BuildKit, and could be a potential candidate
for moving to a separate module. While it's not too problematic to have
this dependency, the tests only used basic assertions from gotest.tools,
which could be easily re-implemented without the dependency.
pkg/reexec: touch-up GoDoc, and remove "import" comments
Touch-up some GoDoc in the package, and remove "import" comments.
This package is used in BuildKit, and could be a potential candidate
for moving to a separate module. The "import" comments are ignored when
used in go module mode so have little benefit. Let's remove them.
- A picture of a cute animal (not mandatory but encouraged)