Added support for Windows. #26
Merged
Conversation
This file contains 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
sergiusens
reviewed
Oct 26, 2021
cmatsuoka
approved these changes
Oct 26, 2021
1 task
flotter
added a commit
to flotter/craft-cli
that referenced
this pull request
Oct 10, 2022
It is a very common pattern for craft tools to start the session directly on a host machine, but launch another craft instance to do the actual work inside LXD or Multipass. In order for a seamless CLI experience (which looks identical between a 'destructive build' on the host, LXD or Multipass), we need support for progress messages (ephemeral=True) to seamlessly cross the boundaries between the host and the container instance of of the craft tool. Please see examples.py canonical#26 for an example. Add support for _Printer to respect ephemeral messages when stopping. On printer stop(), these messages are no longer terminated with a newline, but instead the printer will clear and reset the cursor to the start of the line. This behaviour change will allow subsequent terminal writes by a different process the ability to reuse a terminal line previously used as ephemeral output.
flotter
added a commit
to flotter/craft-cli
that referenced
this pull request
Oct 10, 2022
It is a very common pattern for craft tools to start the session directly on a host machine, but launch another craft instance to do the actual work inside LXD or Multipass. In order for a seamless CLI experience (which looks identical between a 'destructive build' on the host, LXD or Multipass), we need support for progress messages (ephemeral=True) to seamlessly cross the boundaries between the host and the container instance of of the craft tool. Please see examples.py canonical#26 for an example. Add support for _Printer to respect ephemeral messages when stopping. On printer stop(), these messages are no longer terminated with a newline, but instead the printer will clear and reset the cursor to the start of the line. This behaviour change will allow subsequent terminal writes by a different process the ability to reuse a terminal line previously used as ephemeral output.
flotter
added a commit
to flotter/craft-cli
that referenced
this pull request
Oct 10, 2022
It is a very common pattern for craft tools to start the session directly on a host machine, but launch another craft instance to do the actual work inside LXD or Multipass. In order for a seamless CLI experience (which looks identical between a 'destructive build' on the host, LXD or Multipass), we need support for progress messages (ephemeral=True) to seamlessly cross the boundaries between the host and the container instance of of the craft tool. Please see examples.py canonical#26 for an example. Add support for _Printer to respect ephemeral messages when stopping. On printer stop(), these messages are no longer terminated with a newline, but instead the printer will clear and reset the cursor to the start of the line. This behaviour change will allow subsequent terminal writes by a different process the ability to reuse a terminal line previously used as ephemeral output.
tigarmo
pushed a commit
that referenced
this pull request
Jun 6, 2023
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
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.
This includes a functionality change and several tests adjustments (beyond enabling Windows in the test matrix, with the particular case of installing extra dependencies).
The functionality change reflects the fact that there is no "os.pipe" nor "select" in Windows, so:
used
win32pipemodule frompywin32package to create a proper pipe represented as a file descriptor (withFdCreatePipe) so it can be used as a subprocess' "stream"use a totally different loop for reading, which is blocking, so I had to write an extra byte on stopping (to unblock the reading), and support that byte in the read loop
Note 1: all the previous changes are only for "windows mode", which after a little refactor is just enclosed in the _PipeReaderThread class.
Note 2: tests for stream/pipe reading did not really change beyond the refactor just mentioned; "external" functionality is fulfilled just fine, for which I added an "example" suitable to be run in Windows.
The test changes are scattered in several test files, but they are mainly grouped in:
better path representation in the test assert
support for different "time behaviour" (specially in the log filepath timestamp)