Skip to content
This repository has been archived by the owner on Sep 28, 2022. It is now read-only.

Commit

Permalink
[hakari] fix how path dependencies are output
Browse files Browse the repository at this point in the history 
We weren't producing proper TOML, whoops.

Also, no longer silently ignore non-Unicode paths.
  • Loading branch information
@sunshowers
sunshowers committed Feb 4, 2021
1 parent 4a3e1bd commit bf3c802
Show file tree
Hide file tree
Showing 11 changed files with 72 additions and 21 deletions.
2 changes: 1 addition & 1 deletion Cargo.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions fixtures/small/hakari/metadata1-2.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ datatest-derive = { version = "0.4", default-features = false, features = [] }
linked-hash-map = { version = "0.5", default-features = false, features = [] }
memchr = { version = "2", features = ["default", "use_std"] }
proc-macro2 = { version = "1", features = ["default", "proc-macro"] }
quote = { path = "/fakepath/testcrate/../quote"features = ["default", "proc-macro"] }
quote = { path = "/fakepath/testcrate/../quote", features = ["default", "proc-macro"] }
regex = { version = "1", features = ["aho-corasick", "default", "memchr", "perf", "perf-cache", "perf-dfa", "perf-inline", "perf-literal", "std", "thread_local", "unicode", "unicode-age", "unicode-bool", "unicode-case", "unicode-gencat", "unicode-perl", "unicode-script", "unicode-segment"] }
regex-syntax = { version = "0.6", default-features = false, features = ["unicode-age", "unicode-bool", "unicode-case", "unicode-gencat", "unicode-perl", "unicode-script", "unicode-segment"] }
region = { version = "2", default-features = false, features = [] }
Expand Down Expand Up @@ -100,7 +100,7 @@ datatest-derive = { version = "0.4", default-features = false, features = [] }
linked-hash-map = { version = "0.5", default-features = false, features = [] }
memchr = { version = "2", features = ["default", "use_std"] }
proc-macro2 = { version = "1", features = ["default", "proc-macro"] }
quote = { path = "/fakepath/testcrate/../quote"features = ["default", "proc-macro"] }
quote = { path = "/fakepath/testcrate/../quote", features = ["default", "proc-macro"] }
regex = { version = "1", features = ["aho-corasick", "default", "memchr", "perf", "perf-cache", "perf-dfa", "perf-inline", "perf-literal", "std", "thread_local", "unicode", "unicode-age", "unicode-bool", "unicode-case", "unicode-gencat", "unicode-perl", "unicode-script", "unicode-segment"] }
regex-syntax = { version = "0.6", default-features = false, features = ["unicode-age", "unicode-bool", "unicode-case", "unicode-gencat", "unicode-perl", "unicode-script", "unicode-segment"] }
region = { version = "2", default-features = false, features = [] }
Expand Down
4 changes: 2 additions & 2 deletions fixtures/small/hakari/metadata1-3.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ yaml-rust = { version = "0.4", default-features = false, features = [] }
ctor = { version = "0.1", default-features = false, features = [] }
datatest-derive = { version = "0.4", default-features = false, features = [] }
proc-macro2 = { version = "1", features = ["default", "proc-macro"] }
quote = { path = "/fakepath/testcrate/../quote"features = ["default", "proc-macro"] }
quote = { path = "/fakepath/testcrate/../quote", features = ["default", "proc-macro"] }
syn = { version = "1", features = ["clone-impls", "default", "derive", "fold", "full", "parsing", "printing", "proc-macro", "quote"] }
unicode-xid = { version = "0.2", features = ["default"] }
version_check = { version = "0.9", default-features = false, features = [] }
Expand All @@ -71,7 +71,7 @@ yaml-rust = { version = "0.4", default-features = false, features = [] }
ctor = { version = "0.1", default-features = false, features = [] }
datatest-derive = { version = "0.4", default-features = false, features = [] }
proc-macro2 = { version = "1", features = ["default", "proc-macro"] }
quote = { path = "/fakepath/testcrate/../quote"features = ["default", "proc-macro"] }
quote = { path = "/fakepath/testcrate/../quote", features = ["default", "proc-macro"] }
syn = { version = "1", features = ["clone-impls", "default", "derive", "fold", "full", "parsing", "printing", "proc-macro", "quote"] }
unicode-xid = { version = "0.2", features = ["default"] }
version_check = { version = "0.9", default-features = false, features = [] }
Expand Down
2 changes: 1 addition & 1 deletion fixtures/small/hakari/metadata1-5.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ yaml-rust = { version = "0.4", default-features = false, features = [] }
ctor = { version = "0.1", default-features = false, features = [] }
datatest-derive = { version = "0.4", default-features = false, features = [] }
proc-macro2 = { version = "1", features = ["default", "proc-macro"] }
quote = { path = "/fakepath/testcrate/../quote"features = ["default", "proc-macro"] }
quote = { path = "/fakepath/testcrate/../quote", features = ["default", "proc-macro"] }
syn = { version = "1", features = ["clone-impls", "default", "derive", "fold", "full", "parsing", "printing", "proc-macro", "quote"] }
unicode-xid = { version = "0.2", features = ["default"] }
version_check = { version = "0.9", default-features = false, features = [] }
Expand Down
8 changes: 4 additions & 4 deletions fixtures/small/hakari/metadata2-3.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,15 @@ same-file = { version = "1", default-features = false, features = [] }
serde = { version = "1", features = ["default", "std"] }
serde_yaml = { version = "0.8", default-features = false, features = [] }
thread_local = { version = "0.3", default-features = false, features = [] }
walkdir-8f10a84f3ce77303 = { package = "walkdir", path = "/Users/fakeuser/local/testworkspace/../walkdir"default-features = false, features = [] }
walkdir-8f10a84f3ce77303 = { package = "walkdir", path = "/Users/fakeuser/local/testworkspace/../walkdir", default-features = false, features = [] }
walkdir-f595c2ba2a3f28df = { package = "walkdir", version = "2", default-features = false, features = [] }
yaml-rust = { version = "0.4", default-features = false, features = [] }

[target.armv7s-apple-ios.build-dependencies]
ctor = { version = "0.1", default-features = false, features = [] }
datatest-derive = { version = "0.4", default-features = false, features = [] }
proc-macro2 = { version = "1", features = ["default", "proc-macro"] }
quote = { path = "/Users/fakeuser/local/testworkspace/../quote"features = ["default", "proc-macro"] }
quote = { path = "/Users/fakeuser/local/testworkspace/../quote", features = ["default", "proc-macro"] }
syn = { version = "1", features = ["clone-impls", "default", "derive", "fold", "full", "parsing", "printing", "proc-macro", "quote"] }
unicode-xid = { version = "0.2", features = ["default"] }
version_check = { version = "0.9", default-features = false, features = [] }
Expand All @@ -60,15 +60,15 @@ same-file = { version = "1", default-features = false, features = [] }
serde = { version = "1", features = ["default", "std"] }
serde_yaml = { version = "0.8", default-features = false, features = [] }
thread_local = { version = "0.3", default-features = false, features = [] }
walkdir-8f10a84f3ce77303 = { package = "walkdir", path = "/Users/fakeuser/local/testworkspace/../walkdir"default-features = false, features = [] }
walkdir-8f10a84f3ce77303 = { package = "walkdir", path = "/Users/fakeuser/local/testworkspace/../walkdir", default-features = false, features = [] }
walkdir-f595c2ba2a3f28df = { package = "walkdir", version = "2", default-features = false, features = [] }
yaml-rust = { version = "0.4", default-features = false, features = [] }

[target.powerpc-wrs-vxworks-spe.build-dependencies]
ctor = { version = "0.1", default-features = false, features = [] }
datatest-derive = { version = "0.4", default-features = false, features = [] }
proc-macro2 = { version = "1", features = ["default", "proc-macro"] }
quote = { path = "/Users/fakeuser/local/testworkspace/../quote"features = ["default", "proc-macro"] }
quote = { path = "/Users/fakeuser/local/testworkspace/../quote", features = ["default", "proc-macro"] }
syn = { version = "1", features = ["clone-impls", "default", "derive", "fold", "full", "parsing", "printing", "proc-macro", "quote"] }
unicode-xid = { version = "0.2", features = ["default"] }
version_check = { version = "0.9", default-features = false, features = [] }
Expand Down
4 changes: 2 additions & 2 deletions fixtures/small/hakari/metadata2-5.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ regex-syntax = { version = "0.6", default-features = false, features = ["unicode
serde = { version = "1", features = ["default", "std"] }
serde_yaml = { version = "0.8", default-features = false, features = [] }
thread_local = { version = "0.3", default-features = false, features = [] }
walkdir-8f10a84f3ce77303 = { package = "walkdir", path = "/Users/fakeuser/local/testworkspace/../walkdir"default-features = false, features = [] }
walkdir-8f10a84f3ce77303 = { package = "walkdir", path = "/Users/fakeuser/local/testworkspace/../walkdir", default-features = false, features = [] }
walkdir-f595c2ba2a3f28df = { package = "walkdir", version = "2", default-features = false, features = [] }
winapi = { version = "0.3", default-features = false, features = ["consoleapi", "errhandlingapi", "fileapi", "minwindef", "processenv", "std", "winbase", "wincon", "winerror", "winnt"] }
winapi-util = { version = "0.1", default-features = false, features = [] }
Expand All @@ -41,7 +41,7 @@ yaml-rust = { version = "0.4", default-features = false, features = [] }
ctor = { version = "0.1", default-features = false, features = [] }
datatest-derive = { version = "0.4", default-features = false, features = [] }
proc-macro2 = { version = "1", features = ["default", "proc-macro"] }
quote = { path = "/Users/fakeuser/local/testworkspace/../quote"features = ["default", "proc-macro"] }
quote = { path = "/Users/fakeuser/local/testworkspace/../quote", features = ["default", "proc-macro"] }
syn = { version = "1", features = ["clone-impls", "default", "derive", "fold", "full", "parsing", "printing", "proc-macro", "quote"] }
unicode-xid = { version = "0.2", features = ["default"] }
version_check = { version = "0.9", default-features = false, features = [] }
Expand Down
4 changes: 2 additions & 2 deletions fixtures/small/hakari/metadata_targets1-3.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,12 @@
# crates-io = true

[target.armv7s-apple-ios.dependencies]
dep-a = { path = "/Users/fakeuser/local/testcrates/testcrate-targets/../dep-a"default-features = false, features = ["baz", "foo", "quux"] }
dep-a = { path = "/Users/fakeuser/local/testcrates/testcrate-targets/../dep-a", default-features = false, features = ["baz", "foo", "quux"] }
lazy_static-6f8ce4dd05d13bba = { package = "lazy_static", version = "0.2", default-features = false, features = [] }
lazy_static-dff4ba8e3ae991db = { package = "lazy_static", version = "1", default-features = false, features = [] }

[target.powerpc-wrs-vxworks-spe.dependencies]
dep-a = { path = "/Users/fakeuser/local/testcrates/testcrate-targets/../dep-a"default-features = false, features = ["baz", "foo", "quux"] }
dep-a = { path = "/Users/fakeuser/local/testcrates/testcrate-targets/../dep-a", default-features = false, features = ["baz", "foo", "quux"] }
lazy_static-6f8ce4dd05d13bba = { package = "lazy_static", version = "0.2", default-features = false, features = [] }
lazy_static-dff4ba8e3ae991db = { package = "lazy_static", version = "1", default-features = false, features = [] }

Expand Down
4 changes: 2 additions & 2 deletions fixtures/small/hakari/metadata_targets1-6.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,12 @@
# crates-io = true

[target.'cfg(all())'.dependencies]
dep-a = { path = "/Users/fakeuser/local/testcrates/testcrate-targets/../dep-a"default-features = false, features = ["bar", "baz", "foo", "quux"] }
dep-a = { path = "/Users/fakeuser/local/testcrates/testcrate-targets/../dep-a", default-features = false, features = ["bar", "baz", "foo", "quux"] }
lazy_static-c65f7effa3be6d31 = { package = "lazy_static", version = "0.1", default-features = false, features = [] }
lazy_static-dff4ba8e3ae991db = { package = "lazy_static", version = "1", default-features = false, features = [] }

[target.'cfg(all())'.build-dependencies]
dep-a = { path = "/Users/fakeuser/local/testcrates/testcrate-targets/../dep-a"default-features = false, features = ["bar", "baz", "foo", "quux"] }
dep-a = { path = "/Users/fakeuser/local/testcrates/testcrate-targets/../dep-a", default-features = false, features = ["bar", "baz", "foo", "quux"] }
lazy_static-c65f7effa3be6d31 = { package = "lazy_static", version = "0.1", default-features = false, features = [] }
lazy_static-dff4ba8e3ae991db = { package = "lazy_static", version = "1", default-features = false, features = [] }

Expand Down
11 changes: 11 additions & 0 deletions tools/hakari/CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,16 @@
# Changelog

## [0.1.1] - 2021-02-04

### Added

* Experimental Windows support. There may still be bugs around path normalization: please [report them](https://github.com/facebookincubator/cargo-guppy/issues/new).

### Fixed

* Fixed Cargo.toml output for path dependencies.
* Return an error for non-Unicode paths instead of silently producing incorrect paths.

## [0.1.0] - 2021-02-03

Initial release.
Expand Down
2 changes: 1 addition & 1 deletion tools/hakari/Cargo.toml
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "hakari"
version = "0.1.0"
version = "0.1.1"
description = "Manage workspace-hack packages that do feature unification inside workspaces."
documentation = "https://docs.rs/determinator"
authors = ["Rain <[email protected]>"]
Expand Down
48 changes: 44 additions & 4 deletions tools/hakari/src/toml_out.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
use crate::hakari::{HakariBuilder, OutputMap};
#[cfg(feature = "summaries")]
use crate::summaries::HakariBuilderSummary;
use cfg_if::cfg_if;
use guppy::{
graph::{cargo::BuildPlatform, ExternalSource, GitReq, PackageMetadata, PackageSource},
PackageId, TargetSpecError, Version,
Expand Down Expand Up @@ -149,6 +150,17 @@ pub enum TomlOutError {
rel_path: PathBuf,
},

/// A non-Unicode path was encountered while writing out TOML.
///
/// The TOML format only supports Unicode paths.
NonUnicodePath {
/// The package ID that Hakari tried to write out a dependency line for.
package_id: PackageId,

/// The non-Unicode path that Hakari attempted to write.
path: PathBuf,
},

/// An external source wasn't recognized by guppy.
UnrecognizedExternal {
/// The package ID that Hakari tried to write out a dependency line for.
Expand Down Expand Up @@ -178,6 +190,12 @@ impl fmt::Display for TomlOutError {
#[cfg(feature = "summaries")]
TomlOutError::Toml { context, .. } => write!(f, "while serializing TOML: {}", context),
TomlOutError::FmtWrite(_) => write!(f, "while writing to fmt::Write"),
TomlOutError::NonUnicodePath { package_id, path } => write!(
f,
"for path dependency '{}', non-Unicode path encountered: {}",
package_id,
path.display(),
),
TomlOutError::PathWithoutHakari {
package_id,
rel_path,
Expand All @@ -203,9 +221,9 @@ impl error::Error for TomlOutError {
#[cfg(feature = "summaries")]
TomlOutError::Toml { err, .. } => Some(err),
TomlOutError::FmtWrite(err) => Some(err),
TomlOutError::PathWithoutHakari { .. } | TomlOutError::UnrecognizedExternal { .. } => {
None
}
TomlOutError::NonUnicodePath { .. }
| TomlOutError::PathWithoutHakari { .. }
| TomlOutError::UnrecognizedExternal { .. } => None,
}
}
}
Expand Down Expand Up @@ -279,6 +297,8 @@ pub(crate) fn write_toml(
// PackageSource::Workspace shouldn't be possible unless the Hakari map
// was fiddled with. Regardless, we can handle it fine.
let path_out = if options.absolute_paths {
// TODO: canonicalize paths here, removing .. etc? tricky if the path is
// missing (as in tests)
builder.graph().workspace().root().join(path)
} else {
let hakari_path =
Expand All @@ -289,7 +309,27 @@ pub(crate) fn write_toml(
pathdiff::diff_paths(path, hakari_path)
.expect("both hakari_path and path are relative")
};
format!("path = \"{}\"", path_out.display())

let path_str = match path_out.to_str() {
Some(s) => s,
None => {
return Err(TomlOutError::NonUnicodePath {
package_id: dep.id().clone(),
path: path_out,
})
}
};

cfg_if! {
if #[cfg(windows)] {
// TODO: is replacing \\ with / totally safe on Windows? Might run
// into issues with UNC paths.
let path_str = path_str.replace("\\", "/");
format!("path = \"{}\", ", path_str)
} else {
format!("path = \"{}\", ", path_str)
}
}
}
PackageSource::External(s) => {
let external = source.parse_external().ok_or_else(|| {
Expand Down

0 comments on commit bf3c802

Please sign in to comment.