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

feat(docker-build): pass additional workspace directory #1690

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

Conversation

prajwolrg
Copy link

Currently, docker only mounts the guest program workspace. However, mounting a different workspace can be useful. This PR allows passing an additional workspace_directory as BuildArgs.

@prajwolrg prajwolrg force-pushed the feat/docker-optional-workspace-dev branch from 3b81b4c to 648490c Compare November 2, 2024 15:49
@nhtyy nhtyy added the maybe-stale A pr or issue that hasnt seen alot of activity recently label Nov 20, 2024
Copy link
Collaborator

@nhtyy nhtyy left a comment

Choose a reason for hiding this comment

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

Thank you for your patience on this PR!

Happy to take this over, but I think it be suitable to instead override the program dir? Therefore leading to a different workspace metadata?

See here:

let program_dir = program_dir
.unwrap_or_else(|| std::env::current_dir().expect("Failed to get current directory."));
let program_dir: Utf8PathBuf =
program_dir.try_into().expect("Failed to convert PathBuf to Utf8PathBuf");
// Get the program metadata.
let program_metadata_file = program_dir.join("Cargo.toml");
let mut program_metadata_cmd = cargo_metadata::MetadataCommand::new();
let program_metadata = program_metadata_cmd.manifest_path(program_metadata_file).exec()?;
// Get the command corresponding to Docker or local build.
let cmd = if args.docker {
create_docker_command(args, &program_dir, &program_metadata)?
} else {

Currently, this is not exposed as an option via the CLI? Also likely this crate gets cleaned up more in the future so will keep this in mind going forward.

@prajwolrg
Copy link
Author

prajwolrg commented Dec 2, 2024

Thank you for your patience on this PR!

Happy to take this over, but I think it be suitable to instead override the program dir? Therefore leading to a different workspace metadata?

See here:

let program_dir = program_dir
.unwrap_or_else(|| std::env::current_dir().expect("Failed to get current directory."));
let program_dir: Utf8PathBuf =
program_dir.try_into().expect("Failed to convert PathBuf to Utf8PathBuf");
// Get the program metadata.
let program_metadata_file = program_dir.join("Cargo.toml");
let mut program_metadata_cmd = cargo_metadata::MetadataCommand::new();
let program_metadata = program_metadata_cmd.manifest_path(program_metadata_file).exec()?;
// Get the command corresponding to Docker or local build.
let cmd = if args.docker {
create_docker_command(args, &program_dir, &program_metadata)?
} else {

Currently, this is not exposed as an option via the CLI? Also likely this crate gets cleaned up more in the future so will keep this in mind going forward.

In our usecase, we had multiple guest programs in a single workspace. So, we used this approach. We didn't find it exposed via CLI. There seems to have been significant update on build from the last time I looked into this. Happy to help you and review for our usecase if you take it over.

@prajwolrg prajwolrg requested a review from nhtyy December 2, 2024 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maybe-stale A pr or issue that hasnt seen alot of activity recently
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants