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 console-proxy command #288

Merged
merged 14 commits into from
Dec 14, 2022
Merged

Add console-proxy command #288

merged 14 commits into from
Dec 14, 2022

Conversation

jgallagher
Copy link
Contributor

@jgallagher jgallagher commented Dec 9, 2022

humility console-proxy uses the uart control functions added in oxidecomputer/hubris#1005 to do its best (very slow) picocom impersonation.

Before merging:

  • don't try to build this command (or make it report "this isn't supported") on Windows
  • don't allow this command to be used within the repl on non-Windows

Cargo.lock Outdated Show resolved Hide resolved
Cargo.lock Outdated Show resolved Hide resolved
idol = { workspace = true }

[target.'cfg(not(windows))'.dependencies]
termios = "0.3"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a way to make this a conditional workspace dependency?

Copy link
Contributor Author

@jgallagher jgallagher Dec 13, 2022

Choose a reason for hiding this comment

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

Maaaaybe? I don't think we can have target-specific workspace dependencies (e.g., rust-lang/cargo#5220), but maybe we can specify termios = "0.3" as a workspace dep and conditionally depend on it in Cargo.toml. Trying that in 5db1630 to see if it passes Windows CI.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like that worked fine. 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

I had tried doing this in the workspace's cargo.toml and it didn't seem to work. That is, literally this diff, not what @jgallagher just tried. it might!

... is the policy that every single thing becomes a workspace dependency now? I'd personally default to only things that are shared, but I can see wanting to do it all there, too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd lean toward making them all workspace deps, because otherwise if I want to add a new dep, I first ought to grep and see if it's used by something else, and if so, move it to be a workspace dep now that it's becoming shared.

That's slightly annoying but not the end of the world; are there similar annoyances with "everything at the workspace level" I'm not thinking of?

@jgallagher jgallagher enabled auto-merge (squash) December 14, 2022 17:16
@jgallagher jgallagher merged commit 76671be into master Dec 14, 2022
@jgallagher jgallagher deleted the cmd-console-proxy branch December 14, 2022 17:37
bcantrill pushed a commit that referenced this pull request Jun 22, 2023
Co-authored-by: Steve Klabnik <[email protected]>
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.

3 participants