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.

Sign up for GitHub

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

Jump to bottom

add --keep-result-dir flag #132

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

add --keep-result-dir flag #132

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/command/apply.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,12 @@ The built system profiles will be added as GC roots so that they will not be rem
The links will be created under .gcroots in the directory the Hive configuration is located.
"#)
.num_args(0))
.arg(Arg::new("keep-result-dir")
.long("keep-result-dir")
.value_name("DIR")
.help("Path to link gcroots")
.long_help("Path to a directory where the gcroots will be linked")
.default_value(".gcroots"))
.arg(Arg::new("verbose")
.short('v')
.long("verbose")
Expand Down Expand Up @@ -213,6 +219,9 @@ pub async fn run(_global_args: &ArgMatches, local_args: &ArgMatches) -> Result<(

if local_args.get_flag("keep-result") {
options.set_create_gc_roots(true);
if let Some(dir) = local_args.get_one::<String>("keep-result-dir") {
options.set_create_gc_roots_dir(dir.to_owned());
}
}

if local_args.get_flag("no-build-on-target") {
Expand Down
4 changes: 3 additions & 1 deletion src/nix/deployment/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -523,7 +523,9 @@ impl Deployment {
job.run_waiting(|job| async move {
if let Some(dir) = arc_self.hive.context_dir() {
job.state(JobState::Running)?;
let path = dir.join(".gcroots").join(format!("node-{}", &*target.name));
let path = dir
.join(self.options.create_gc_roots_dir.to_owned())
Copy link
Owner

Choose a reason for hiding this comment

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

When people specify --keep-result-dir, they might expect the path to be relative to the PWD instead of the configuration base. I'm not sure what's the desirable behavior here.

Copy link
Author

Choose a reason for hiding this comment

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

isn't the behavior still the same as before? Not sure I understand what the configuration base is. The default is still to put things into .gcroots in whatever directory you are in currently, but now you may also specify other absolute or relative paths.

Copy link
Owner

Choose a reason for hiding this comment

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

The thing is that Colmena doesn't put it in .gcroots under whatever directory you are in, but under the directory where flake.nix or hive.nix exists. If no -f is specified, Colmena automatically searched upwards for a flake.nix or hive.nix and use it as the context_dir. With the current implementation, the behavior of colmena apply --keep-result --keep-result-dir ./.myroots would be confusing if it's run in a sub-directory.

Copy link
Author

Choose a reason for hiding this comment

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

Ah, I see... I'm not sure what people would expect since I never change into subdirectories, but one could always give --keep-result-dir "$PWD/.myroots" in this case?

.join(format!("node-{}", &*target.name));

profile_r.create_gc_root(&path).await?;
} else {
Expand Down
7 changes: 7 additions & 0 deletions src/nix/deployment/options.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ pub struct Options {
/// directory if it exists.
pub(super) create_gc_roots: bool,

pub(super) create_gc_roots_dir: String,

/// Whether to override per-node setting to build on the nodes themselves.
pub(super) force_build_on_target: Option<bool>,

Expand Down Expand Up @@ -63,6 +65,10 @@ impl Options {
self.create_gc_roots = enable;
}

pub fn set_create_gc_roots_dir(&mut self, dir: String) {
self.create_gc_roots_dir = dir;
}

pub fn set_force_build_on_target(&mut self, enable: bool) {
self.force_build_on_target = Some(enable);
}
Expand Down Expand Up @@ -92,6 +98,7 @@ impl Default for Options {
upload_keys: true,
reboot: false,
create_gc_roots: false,
create_gc_roots_dir: String::from(".gcroots"),
force_build_on_target: None,
force_replace_unknown_profiles: false,
evaluator: EvaluatorType::Chunked,
Expand Down