Skip to content

xtask: migrating from duct to xshell #251

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

Merged
merged 1 commit into from
Feb 29, 2024

Conversation

gabriele-0201
Copy link
Contributor

@gabriele-0201 gabriele-0201 commented Feb 22, 2024

Duct exhibits too many peculiar behaviors:

  • Regarding how it deals with relative paths, it seems that the spawned process assumes a position in the file system relative to where the parent process was spawned, not where the child process was spawned.
  • You cannot use the same opened file in multiple commands because they take ownership of the file descriptor and close it once the command finishes.
  • If you capture stderr to print it later but the command fails with a non-zero status code, it will drop the stderr and return an empty enum. Stdout and stderr are only returned if the command terminates correctly.
  • There is no way to convert duct::Cmd to a std::process::Command that is needed to deal with process group IDs.

That's why I am moving to xshell. However, it does not handle async, so the addition of Tokio is required. Moving everything to Tokio raises all the problems related to issue #228. I handled them using the approach that @pepyakin and I came up with. Zombie processes should not be a problem anymore

closes #228

Copy link
Contributor Author

gabriele-0201 commented Feb 22, 2024

Current dependencies on/for this PR:

This stack of pull requests is managed by Graphite.

@gabriele-0201 gabriele-0201 force-pushed the gab_migrate_to_xshell_tokio branch 4 times, most recently from cffa541 to 2951e12 Compare February 26, 2024 17:38
@gabriele-0201 gabriele-0201 marked this pull request as ready for review February 26, 2024 17:44
@gabriele-0201 gabriele-0201 force-pushed the gab_migrate_to_xshell_tokio branch 7 times, most recently from 7207ec0 to 4dbdc69 Compare February 27, 2024 18:21
@gabriele-0201 gabriele-0201 changed the base branch from main to gab_ci_print_log_and_cache_clean February 27, 2024 19:12
@gabriele-0201 gabriele-0201 force-pushed the gab_migrate_to_xshell_tokio branch from 4dbdc69 to c1ca3ac Compare February 27, 2024 19:12
@gabriele-0201 gabriele-0201 force-pushed the gab_ci_print_log_and_cache_clean branch from 76b2104 to 8e8ed0e Compare February 27, 2024 19:22
@gabriele-0201 gabriele-0201 force-pushed the gab_migrate_to_xshell_tokio branch from c1ca3ac to 440a1e0 Compare February 27, 2024 19:23
@gabriele-0201 gabriele-0201 force-pushed the gab_ci_print_log_and_cache_clean branch from 8e8ed0e to d2ef598 Compare February 27, 2024 19:38
@gabriele-0201 gabriele-0201 force-pushed the gab_migrate_to_xshell_tokio branch from 440a1e0 to f94b837 Compare February 27, 2024 19:38
let sov_demo_rollup_path = self.project_path.join("demo/sovereign/demo-rollup/");
sh.change_dir(sov_demo_rollup_path);
Copy link
Contributor

Choose a reason for hiding this comment

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

this approach looks much more robust

@gabriele-0201 gabriele-0201 force-pushed the gab_migrate_to_xshell_tokio branch from f94b837 to 4dd0fee Compare February 28, 2024 17:34
@gabriele-0201 gabriele-0201 force-pushed the gab_ci_print_log_and_cache_clean branch from d2ef598 to adb8339 Compare February 28, 2024 17:39
@gabriele-0201 gabriele-0201 force-pushed the gab_migrate_to_xshell_tokio branch from 4dd0fee to f556561 Compare February 28, 2024 17:39
xshell does not handle async, so it also requires the addition of Tokio.
process group IDs are used to kill all xtask subprocesses
@gabriele-0201 gabriele-0201 changed the base branch from gab_ci_print_log_and_cache_clean to gab_ci_print_log February 29, 2024 14:12
@gabriele-0201 gabriele-0201 force-pushed the gab_migrate_to_xshell_tokio branch from f556561 to 3147dd3 Compare February 29, 2024 14:12
@pepyakin pepyakin merged commit 5080ff6 into gab_ci_print_log Feb 29, 2024
@pepyakin pepyakin deleted the gab_migrate_to_xshell_tokio branch February 29, 2024 14:49
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