Part 3 of RFC2906 - Add support for inheriting `license-path`, and `depednency.p...
source link: https://github.com/rust-lang/cargo/pull/10538
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.
Outdated Show resolved
Outdated Show resolved
Outdated Show resolved
Outdated
@@ -1021,6 +1022,12 @@ impl<T> MaybeWorkspace<T> {
)),
}
}
fn get_defined(&self) -> CargoResult<&T> {
match self {
MaybeWorkspace::Workspace(_) => Err(anyhow!("MaybeWorkspace was Workspace")),
nit: I think I'd just do this as an as_defined(&self) -> Option<&T> and let the error message be completely controlled by the caller
I could go either way on this. If you feel its better I'll change it to something like this
Outdated Show resolved
Outdated
if let Some(diff_str) = diff_path.to_str() {
self.path = Some(diff_str.to_owned());
}
}
If I'm reading this correctly, we are leaving the incorrect relative path in if it doesn't work?
Wonder if we should have a function shared between the the path rewriting so we can be consistent on error reporting.
You mean something like
pub fn resolve_relative_path(label: &str, root_path: Pathbuf, package_root: PathBuf) -> CargoResult<String> {
match diff_paths(root_path, package_root) {
None => Err(anyhow!("`workspace.{}` was defined but could not be resolved with {}", label, package_root.display())),
Some(path) => Ok(
path
.to_str()
.ok_or_else(|| anyhow!("`{}` resolved to non-UTF value (`{}`)", label, path.display()))?
.to_owned()
),
}
}- I'd recommend having
old_root,new_root, andrel_pathas parameters so it can also handlejoin+normalize_pathfor you workspace.doesn't quite work for path dependencies. Maybe just leave it as{label}and re-word to the first to include both paths
pub fn resolve_relative_path(
label: &str,
old_root: &PathBuf,
new_root: &PathBuf,
rel_path: &str,
) -> CargoResult<String> {
let joined_path = normalize_path(&old_root.join(rel_path));
match diff_paths(joined_path, new_root) {
None => Err(anyhow!(
"`{}` was defined in {} but could not be resolved with {}",
label,
old_root.display(),
package_root.display()
)),
Some(path) => Ok(path
.to_str()
.ok_or_else(|| {
anyhow!(
"`{}` resolved to non-UTF value (`{}`)",
label,
path.display()
)
})?
.to_owned()),
}
}Seems like that'd work
Outdated
fn resolve_path(&mut self, root_path: &Path, package_root: &Path, config: &Config) {
if let Some(from_root) = &self.path {
let resolved = from_root.resolve(config);
if let Some(diff_path) = diff_paths(root_path.join(resolved), package_root) {
Does diff_paths gracefully handle diffing /foo/bar/../baz with /foo/bee? Do we need to call cargo_utils::paths::normlize_path after doing the join?
I remember having to be particular about that in cargo-add but I don't remember if it was related to diff_paths or other stuff
(maybe another reason for a shared function, see below)
I don't know if it handles that well or not. I have a feeling that we will need to call cargo_utils::paths::normlize_path as well. I'll change it to this.
"license": null,
"license_file": null,
"license": "MIT",
"license_file": "[..]LICENSE",
I feel like we'll get a better idea if the tests are doing what we expect if they were to use [ROOT] instead of [..]
I'll change it to [ROOT], I agree that it gives a better idea of what is going on
I tried to do this and it didn't work for some reason. I just changed it for ../LICENSE
Oh right, its relative now. Thats what I'm looking for, thanks!
}
impl TomlDependency {
Why the new block?
Same reasons as for DetailedTomlDependency
Outdated Show resolved
Recommend
About Joyk
Aggregate valuable and interesting links.
Joyk means Joy of geeK