Use Verify to check required commands are present #598
Conversation
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
daissi
force-pushed
the
eds/daissi/verify
branch
4 times, most recently
from
e4d690a to
85ed64c
Compare
obbardc
requested changes
Oct 8, 2025
| return | ||
| } | ||
|
|
||
| if !runInFakeMachine && !fakemachine.InMachine() { |
Member
There was a problem hiding this comment.
are these few lines a merge error ? they dont look like they belong here
| } | ||
|
|
||
| if !runInFakeMachine { | ||
| // Either we're on the host and intend to run on it, |
Member
There was a problem hiding this comment.
can you just call CheckEnvironment independently of all of this ? in same area as Verify ?
daissi
force-pushed
the
eds/daissi/verify
branch
from
85ed64c to
399024c
Compare
Signed-off-by: Dylan Aïssi <dylan.aissi@collabora.com>
Verify (which should be called Prepare) must run before the fakemachine is created, in order to setup the arguments for the fakemachine itself. It then has to run again in the fakemachine in order to rebuild any necessary internal state. Command checking can only occur in the same environment as run, as commands that are in the path when running may not be available to the regular user (e.g. Debian installs debootstrap in sbin). Signed-off-by: Ed Smith <ed.smith@collabora.com> Signed-off-by: Dylan Aïssi <dylan.aissi@collabora.com>
daissi
force-pushed
the
eds/daissi/verify
branch
from
399024c to
7010a53
Compare
sjoerdsimons
requested changes
Oct 24, 2025
Member
sjoerdsimons
left a comment
sjoerdsimons
left a comment
There was a problem hiding this comment.
Need to verify the mmdebstrap action as well
Member
|
@daissi is there a reason for this to be a draft? |
Signed-off-by: Ed Smith <ed.smith@collabora.com> Signed-off-by: Dylan Aïssi <dylan.aissi@collabora.com>
daissi
force-pushed
the
eds/daissi/verify
branch
from
7010a53 to
e7f54b5
Compare
Member
Author
Fixed! |
Member
Author
Just because I haven't addressed all the comments, but let's remove the draft status. |
Member
|
@daissi this needs rebasing on main & couple of simple comments actioning :-) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
For each action, check that as many commands it needs are available as possible. This can't be meaningfully extended to commands that run in the chroot, or might run in the chroot.
This follows up from comments on #276 which were captured as issue #280.
Closes: #280
Based on: #283