-
Notifications
You must be signed in to change notification settings - Fork 63
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
Support workspaces at library level #197
Merged
Merged
Changes from 11 commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
38034bc
Bump openssl from 0.10.45 to 0.10.48 (#160)
dependabot[bot] 149af2c
Merge branch 'Wulf:main' into main
AnthonyMichaelTDM e65675f
support using workspaces with a virtual manifest at the root Cargo.to…
AnthonyMichaelTDM c7fcd03
clippy and fmt
AnthonyMichaelTDM e32badb
CI.yml optimizations from #182
AnthonyMichaelTDM b15eb73
clippy and fmt fixes from #182 and elsewhere
AnthonyMichaelTDM 201ca5e
Add workspace support feature
AnthonyMichaelTDM f23fd1f
some additional comments
AnthonyMichaelTDM 59eb497
Merge branch 'main' into support-workspaces
AnthonyMichaelTDM 5d1f900
support project organization as a workspace
AnthonyMichaelTDM be8ff07
Merge branch 'main' into support-workspaces
AnthonyMichaelTDM 49c2536
don't rely on feature flag in create-rust-app/src/dev/mod.rs
AnthonyMichaelTDM bdd2020
refactor workspace_utils values to static strings
AnthonyMichaelTDM 9e32fd3
refactor workspace_utils vars so we can remove the feature flag if we…
AnthonyMichaelTDM 81a92fa
Merge branch 'main' into support-workspaces
AnthonyMichaelTDM 2d42480
remove need for plugin_worspace_support feature flag and overhaul var…
AnthonyMichaelTDM 0eaf0a2
refactor and test logic in workspace_utils.rs
AnthonyMichaelTDM File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -101,4 +101,4 @@ jobs: | |
- uses: actions-rs/[email protected] | ||
with: | ||
command: fmt | ||
args: --all -- --check | ||
args: --all -- --check |
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
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
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
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
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
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
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,84 @@ | ||
use std::path::{Path, PathBuf}; | ||
|
||
use lazy_static::lazy_static; | ||
|
||
lazy_static!( | ||
pub static ref WORKSPACE_DIR: PathBuf = { | ||
let output = std::process::Command::new(env!("CARGO")) | ||
.arg("locate-project") | ||
.arg("--workspace") | ||
.arg("--message-format=plain") | ||
.output() | ||
.unwrap() | ||
.stdout; | ||
let cargo_path = Path::new(std::str::from_utf8(&output).unwrap().trim()); | ||
cargo_path.parent().unwrap().to_path_buf() | ||
// .to_str() | ||
// .unwrap() | ||
// .to_owned() | ||
}; | ||
/// constant for the path to the project's frontend directory | ||
pub(crate) static ref FRONTEND_DIR: String = { | ||
match std::env::var("CRA_FRONTEND_DIR") { | ||
Ok(dir) => dir, | ||
Err(_) => { | ||
#[cfg(not(feature = "plugin_workspace_support"))] | ||
{ | ||
"./frontend".to_string() | ||
} | ||
#[cfg(feature = "plugin_workspace_support")] | ||
{ | ||
if *WORKSPACE_DIR == std::env::current_dir().unwrap() { | ||
// this is for when cargo run is run from the workspace root | ||
return "./frontend".to_string(); | ||
} else { | ||
// this is for when cargo run is run from teh backend directory | ||
return "../frontend".to_string(); | ||
} | ||
} | ||
} | ||
} | ||
}; | ||
/// constant for the path to the project's manifest.json file | ||
pub(crate) static ref MANIFEST_PATH: String = { | ||
match std::env::var("CRA_MANIFEST_PATH") { | ||
Ok(dir) => dir, | ||
Err(_) => { | ||
#[cfg(not(feature = "plugin_workspace_support"))] | ||
{ | ||
"./frontend/dist/manifest.json".to_string() | ||
} | ||
#[cfg(feature = "plugin_workspace_support")] | ||
{ | ||
if *WORKSPACE_DIR == std::env::current_dir().unwrap() { | ||
return "./frontend/dist/manifest.json".to_string(); | ||
} else { | ||
return "../frontend/dist/manifest.json".to_string(); | ||
} | ||
} | ||
} | ||
} | ||
}; | ||
/// constant for the path to the project's views directory | ||
pub(crate) static ref VIEWS_GLOB: String = { | ||
match std::env::var("CRA_VIEWS_GLOB") { | ||
Ok(dir) => dir, | ||
Err(_) => { | ||
#[cfg(not(feature = "plugin_workspace_support"))] | ||
{ | ||
"backend/views/**/*.html".to_string() | ||
} | ||
#[cfg(feature = "plugin_workspace_support")] | ||
{ | ||
if *WORKSPACE_DIR == std::env::current_dir().unwrap() { | ||
return "backend/views/**/*.html".to_string(); | ||
} else { | ||
return "views/**/*.html".to_string(); | ||
} | ||
} | ||
} | ||
} | ||
}; | ||
|
||
|
||
); |
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the rest looks good to me but here we're assuming we'll have the project's source code. That's not necessarily the case (see #332 -- we split the build and binary container into two). We also don't want to go in this direction as it stops us from attempting a "single binary" build in the future as well.
It does point out that there's some existing tech debt here that needs to be addressed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we have it stored in an environment variable instead? Similar to frontenddir
It's just that we need a way to know where the base of the project is and that's not always the current working directory
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah. Some more thoughts:
I think we need to remove the feature flag. In
create-rust-app/src/dev/mod.rs
, we should check if it's a workspace using theCargo.toml
instead of relying on a feature flag.Also, all the variables in this file should be
&'static str
and be initialized to some default unless the backing environment variable is present (and they should all be configurable by env vars).Let me know what you think!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've implemented most of your suggestions as of the most recent commits (also got rid of some unnecessary unwraps), but there's a few things I still need to figure out:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
btw, sorry I've been awol for so long, I was busy at work over the summer and then school's been keeping me busy too, so I haven't had much time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sounds awesome -- it's cool to hear about your journey so far :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll have to take some time out to look into this further
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I figured something out, high level is that I made "finding the workspace dir" infallible (no panics, even in a container or something), then rethought the logic used in initializing these variables (getting rid of the need for the feature flag)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Wulf is this resolved?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
beautifully done.
🙌