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

Add flag to preserve build tree for interactive inspection #60

Closed
wants to merge 1 commit into from

Conversation

msehnout
Copy link
Contributor

@msehnout msehnout commented Aug 5, 2019

In case the stage or assembler fails, the build tree is cleaned up
automatically, this option allow us to examine the tree in interactive
bash before it gets removed.

Copy link
Contributor

@larskarlitski larskarlitski left a comment

Choose a reason for hiding this comment

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

Interesting! I see two problems:

  1. The keepalive option doesn't work when check=False.
  2. It won't work at all if bash isn't (yet) installed

I wonder if we should just save the tree on failure and print its id?

@msehnout
Copy link
Contributor Author

msehnout commented Aug 7, 2019

Interesting! I see two problems:

1. The `keepalive` option doesn't work when `check=False`.

Yes, but I can't see any usage of check=False in the code? Maybe we can force check=True when keepalive=True?

2. It won't work at all if `bash` isn't (yet) installed

I wonder if we should just save the tree on failure and print its id?

Yes, that would be nice. Otherwise I would just let osbuild fail in case bash is not yet installed. We can fix it once there is a problem with it.

In case the stage or assembler fails, the build tree is cleaned up
automatically, this option allow us to examine the tree in interactive
bash before it gets removed.
@teg
Copy link
Member

teg commented Aug 12, 2019

This goes a bit in the direction of the debug mode we had before (trying to allow you to enter the namespace using machinectl). I think something like this is useful, but I don't think we should spawn bash. Instead, we should simply wait for the user to Ctrl-C the failed stage, and give them the information needed to access the tree from the outside (its path). It may also make sense to pin the relevant namespaces as files in some discoverable place so that we could make a tool to introspect them. Details are still fuzzy to me, but take home is: don't rely on bash being in the target, and don't spawn bash at all, just give the path of the tree for the caller (or some secondary tool we make) to do with what they wil.

@msehnout
Copy link
Contributor Author

Sure, we don't need to use this approach. It is just something I used when I was debugging my stages and I wanted to share the idea, but I'm all in for something more generally useful.

@teg
Copy link
Member

teg commented Aug 13, 2019

Makes sense! I'll try to file another PR along the lines of what I outlined above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants