Skip to content

--progress-fd being hidden makes it unusable #1181

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

Closed
antheas opened this issue Mar 8, 2025 · 6 comments
Closed

--progress-fd being hidden makes it unusable #1181

antheas opened this issue Mar 8, 2025 · 6 comments

Comments

@antheas
Copy link
Contributor

antheas commented Mar 8, 2025

If --progress-fd is hidden in --help, it is not possible for programs to query if bootc has progress support. So the option cannot be used.

I missed cli: Make --progress-fd hide=true for now.

So, I cannot migrate us off of the PR into mainline bootc. If i take it on faith that bootc accepts --progress-fd you cannot remove it anymore

It is also not possible to use the 0 and 1 fds anymore for debugging in cli.

If you did not like the syntax, leaving it as --json-fd, marked as experimental but not hidden, would have made it easier for you to deprecate and remove.

@antheas
Copy link
Contributor Author

antheas commented Mar 8, 2025

You also made the API stateful which makes it so that if the program misses the version packet it can no longer trust the future ones. But thats a smaller nit I was fine with.

@cgwalters
Copy link
Collaborator

If --progress-fd is hidden in --help, it is not possible for programs to query if bootc has progress support. So the option cannot be used.

Well, one could parse the output of bootc --version, which isn't explicitly documented to be machine-readable today, but would clearly work for this right?

(Tangent, it probably makes sense to add a --version-yaml or so (like what ostree --version does, including the feature set; or maybe we just do what systemctl --version does and include features in a secondary line)

If you did not like the syntax, leaving it as --json-fd, marked as experimental but not hidden, would have made it easier for you to deprecate and remove.

Hey so what happened here is basically we needed to get a release out, but I was a little uncertain about committing to that exact progress format ~forever. But it was my bad that I didn't get back to looking at the tasks to mark it officially stable and left it.

I think we had rough consensus on the overall idea.

BTW, #1016 is linked as the tracker for stabilization, so this discussion is probably better over there.

You also made the API stateful which makes it so that if the program misses the version packet it can no longer trust the future ones. But thats a smaller nit I was fine with.

How could it get missed?

@cgwalters
Copy link
Collaborator

Well, one could parse the output of bootc --version, which isn't explicitly documented to be machine-readable today, but would clearly work for this right?

Also though it seems like in your case (and this is the case in general for custom bootc operating systems) - you're in control of the version of both bootc and your code, so it certainly seems like you could just ensure that your container build has a new enough bootc, right?

@antheas
Copy link
Contributor Author

antheas commented Mar 8, 2025

Well, one could parse the output of bootc --version, which isn't explicitly documented to be machine-readable today, but would clearly work for this right?

Also though it seems like in your case (and this is the case in general for custom bootc operating systems) - you're in control of the version of both bootc and your code, so it certainly seems like you could just ensure that your container build has a new enough bootc, right?

Not exactly. We have testing and stable branches so it would need some coordination for it to be bumped in both correctly. Not impossible but I would rather avoid it

The problem is that if I pass --progress-fd to bootc and it does not support it I cannot exactly fallback to the non fd version otherwise there'd be cases I try to do updates twice

So we risk breaking the updater which requires manual intervention

@antheas
Copy link
Contributor Author

antheas commented Mar 8, 2025

If --progress-fd is hidden in --help, it is not possible for programs to query if bootc has progress support. So the option cannot be used.

Well, one could parse the output of bootc --version, which isn't explicitly documented to be machine-readable today, but would clearly work for this right?

Version check is fragile, especially if you end up removing the parameter in a future version. The updater would break. With the --help check we ensure --progress-fd works so bootc will not bail. Even if the protocol changes, only the progress bar will break.

(Tangent, it probably makes sense to add a --version-yaml or so (like what ostree --version does, including the feature set; or maybe we just do what systemctl --version does and include features in a secondary line)

Yaml parsers are too complicated unfortunately. JSON would be better in this case. I get what you are saying. bootc status already has an API version that is machine readable and I do check that before the overall updater is enabled.

If you did not like the syntax, leaving it as --json-fd, marked as experimental but not hidden, would have made it easier for you to deprecate and remove.

Hey so what happened here is basically we needed to get a release out, but I was a little uncertain about committing to that exact progress format ~forever. But it was my bad that I didn't get back to looking at the tasks to mark it officially stable and left it.

I think we had rough consensus on the overall idea.

BTW, #1016 is linked as the tracker for stabilization, so this discussion is probably better over there.

You also made the API stateful which makes it so that if the program misses the version packet it can no longer trust the future ones. But thats a smaller nit I was fine with.

How could it get missed?

The problem here is not missing per say. Moreso that the message cannot be skipped.

Consider an updater that starts bootc and then due to some loading of the progress bar, etc, ends up introducing a 3-5s delay before reading the progress output. In this case, it would delay the update starting by 5 seconds. E.g., I already have an idle loop in my application that could have been used for the updater, but I had to spin a new thread to ensure I do not block bootc.

@antheas
Copy link
Contributor Author

antheas commented Mar 8, 2025

Let's close this and track on the main tracking issue

@antheas antheas closed this as completed Mar 8, 2025
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 a pull request may close this issue.

2 participants