Skip to content

Commit

Permalink
Fix CRT and ATL casing (#74)
Browse files Browse the repository at this point in the history
* Fix bug where debug symbols were splatted

* Fix diff output

* Update deterministic

* Add symlinks for SDK headers

CRT and ATL can include SDK headers...sigh

* Update deps

* Clippy

* Update snapshots
  • Loading branch information
Jake-Shadle authored Mar 6, 2023
1 parent 948cbad commit 685924a
Show file tree
Hide file tree
Showing 12 changed files with 452 additions and 369 deletions.
558 changes: 263 additions & 295 deletions Cargo.lock

Large diffs are not rendered by default.

16 changes: 2 additions & 14 deletions deny.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,7 @@ ignore = []

[licenses]
unlicensed = "deny"
allow = [
"MIT",
"Apache-2.0",
"ISC",
# This is not really a license for source code, but the encoding* crates
# pulled in by msi all use it for some reason
"CC0-1.0",
]
allow = ["MIT", "Apache-2.0", "BSD-3-Clause", "ISC"]
copyleft = "deny"
exceptions = [
{ allow = ["MPL-2.0"], name = "webpki-roots" },
Expand All @@ -40,15 +33,10 @@ name = "ring"
expression = "ISC AND MIT AND OpenSSL"
license-files = [{ path = "LICENSE", hash = 0xbd0eed23 }]

[[licenses.clarify]]
name = "encoding_rs"
expression = "(Apache-2.0 OR MIT) AND BSD-3-Clause"
license-files = [{ path = "COPYRIGHT", hash = 0x39f8ad31 }]

[bans]
multiple-versions = "deny"
deny = []
skip = [{ name = "terminal_size", version = "=0.1.17" }]
skip = []

[sources]
unknown-registry = "deny"
Expand Down
21 changes: 16 additions & 5 deletions src/ctx.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::{
splat::SdkHeaders,
util::{ProgressTarget, Sha256},
Path, PathBuf, WorkItem,
};
Expand Down Expand Up @@ -204,10 +205,12 @@ impl Ctx {
};

let mut results = Vec::new();
let crt_ft = parking_lot::Mutex::new(None);
let atl_ft = parking_lot::Mutex::new(None);

payloads
.into_par_iter()
.map(|wi| -> Result<Option<crate::splat::SdkHeaders>, Error> {
.map(|wi| -> Result<Option<SdkHeaders>, Error> {
let payload_contents =
crate::download::download(self.clone(), packages.clone(), &wi)?;

Expand All @@ -226,7 +229,7 @@ impl Ctx {
config,
splat_roots.as_ref().unwrap(),
&wi,
ft,
&ft,
arches,
variants,
)
Expand All @@ -235,6 +238,12 @@ impl Ctx {
None
};

match wi.payload.kind {
crate::PayloadKind::CrtHeaders => *crt_ft.lock() = Some(ft),
crate::PayloadKind::AtlHeaders => *atl_ft.lock() = Some(ft),
_ => {}
}

Ok(sdk_headers)
})
.collect_into_vec(&mut results);
Expand All @@ -244,7 +253,10 @@ impl Ctx {

if let Some(roots) = splat_roots {
if enable_symlinks {
crate::splat::finalize_splat(&self, &roots, sdk_headers)?;
let crt_ft = crt_ft.lock().take();
let atl_ft = atl_ft.lock().take();

crate::splat::finalize_splat(&self, &roots, sdk_headers, crt_ft, atl_ft)?;
}
}

Expand Down Expand Up @@ -300,8 +312,7 @@ impl Ctx {
unpack_dir.push(".unpack");
let um = serde_json::to_vec(&um)?;

std::fs::write(&unpack_dir, &um)
.with_context(|| format!("unable to write {unpack_dir}"))?;
std::fs::write(&unpack_dir, um).with_context(|| format!("unable to write {unpack_dir}"))?;
Ok(())
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/download.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ fn download_cabs(
.map(
|(cab_name, chksum, url, sequence)| -> Result<CabContents, Error> {
let cab_contents =
ctx.get_and_validate(&url, &cab_name, Some(chksum), msi.progress.clone())?;
ctx.get_and_validate(url, &cab_name, Some(chksum), msi.progress.clone())?;
Ok(CabContents {
path: cab_name,
content: cab_contents,
Expand Down
113 changes: 85 additions & 28 deletions src/splat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ pub(crate) fn splat(
config: &SplatConfig,
roots: &SplatRoots,
item: &crate::WorkItem,
tree: crate::unpack::FileTree,
tree: &crate::unpack::FileTree,
arches: u32,
variants: u32,
) -> Result<Option<SdkHeaders>, Error> {
Expand Down Expand Up @@ -369,24 +369,25 @@ pub(crate) fn splat(
// want to show that we processed them all
item.progress.inc(*size);

let fname_str = fname.as_str();
if mapping.kind == PayloadKind::CrtLibs || mapping.kind == PayloadKind::Ucrt {
if !include_debug_symbols && fname.ends_with(".pdb") {
tracing::debug!("skipping {fname}");
continue;
}
if !include_debug_symbols && fname.extension() == Some("pdb") {
tracing::debug!("skipping {fname}");
continue;
}

if !include_debug_libs {
if let Some(stripped) = fname_str.strip_suffix(".lib") {
if stripped.ends_with('d')
|| stripped.ends_with("d_netcore")
|| stripped
.strip_suffix(|c: char| c.is_ascii_digit())
.map_or(false, |fname| fname.ends_with('d'))
{
tracing::debug!("skipping {fname}");
continue;
}
let fname_str = fname.as_str();
if !include_debug_libs
&& (mapping.kind == PayloadKind::CrtLibs
|| mapping.kind == PayloadKind::Ucrt)
{
if let Some(stripped) = fname_str.strip_suffix(".lib") {
if stripped.ends_with('d')
|| stripped.ends_with("d_netcore")
|| stripped
.strip_suffix(|c: char| c.is_ascii_digit())
.map_or(false, |fname| fname.ends_with('d'))
{
tracing::debug!("skipping {fname}");
continue;
}
}
}
Expand Down Expand Up @@ -612,6 +613,8 @@ pub(crate) fn finalize_splat(
ctx: &Ctx,
roots: &SplatRoots,
sdk_headers: Vec<SdkHeaders>,
crt_headers: Option<crate::unpack::FileTree>,
atl_headers: Option<crate::unpack::FileTree>,
) -> Result<(), Error> {
let mut files: std::collections::HashMap<
_,
Expand Down Expand Up @@ -640,7 +643,8 @@ pub(crate) fn finalize_splat(
}
}

let mut includes: std::collections::HashSet<
let mut includes: std::collections::HashMap<
_,
_,
std::hash::BuildHasherDefault<twox_hash::XxHash64>,
> = Default::default();
Expand All @@ -663,7 +667,7 @@ pub(crate) fn finalize_splat(
}

rp.contains(|c: char| c.is_ascii_uppercase())
.then(|| PathBuf::from(rp.to_ascii_lowercase()))
.then(|| (PathBuf::from(rp.to_ascii_lowercase()), true))
})
}));

Expand All @@ -680,7 +684,7 @@ pub(crate) fn finalize_splat(
);

pb.set_prefix("symlinks");
pb.set_message("🔍 includes");
pb.set_message("🔍 SDK includes");

// Scan all of the files in the include directory for includes so that
// we can add symlinks to at least make the SDK headers internally consistent
Expand All @@ -701,17 +705,71 @@ pub(crate) fn finalize_splat(
// use incorrect `\` path separators, this is hopefully not an issue
// since no one cares about that target? But if it is a problem
// we'll need to actually modify the include to fix the path. :-/
if !includes.contains(Path::new(rel_path)) {
includes.insert(PathBuf::from(rel_path));
if !includes.contains_key(Path::new(rel_path)) {
includes.insert(PathBuf::from(rel_path), true);
}
}

pb.inc(1);
}

if let Some(crt) = crt_headers
.as_ref()
.and_then(|crt| crt.subtree(Path::new("include")))
{
pb.set_message("🔍 CRT includes");
let cr = roots.crt.join("include");

for (path, _) in &crt.files {
// Of course, there are files with non-utf8 encoding :p
let path = cr.join(path);
let contents =
std::fs::read(&path).with_context(|| format!("unable to read CRT {path}"))?;

for caps in regex.captures_iter(&contents) {
let rel_path = std::str::from_utf8(&caps[1]).with_context(|| {
format!("{path} contained an include with non-utf8 characters")
})?;

if !includes.contains_key(Path::new(rel_path)) {
includes.insert(PathBuf::from(rel_path), false);
}
}

pb.inc(1);
}
}

if let Some(atl) = atl_headers
.as_ref()
.and_then(|atl| atl.subtree(Path::new("include")))
{
pb.set_message("🔍 ATL includes");
let cr = roots.crt.join("include");

for (path, _) in &atl.files {
// Of course, there are files with non-utf8 encoding :p
let path = cr.join(path);
let contents =
std::fs::read(&path).with_context(|| format!("unable to read ATL {path}"))?;

for caps in regex.captures_iter(&contents) {
let rel_path = std::str::from_utf8(&caps[1]).with_context(|| {
format!("{path} contained an include with non-utf8 characters")
})?;

if !includes.contains_key(Path::new(rel_path)) {
includes.insert(PathBuf::from(rel_path), false);
}
}

pb.inc(1);
}
}

pb.finish();

for include in includes {
for (include, is_sdk) in includes {
let lower_hash = calc_lower_hash(include.as_str());

match files.get(&lower_hash) {
Expand All @@ -725,10 +783,9 @@ pub(crate) fn finalize_splat(
_ => {}
},
None => {
tracing::debug!(
"SDK include for '{}' was not found in the SDK headers",
include
);
if is_sdk {
tracing::debug!("SDK include for '{include}' was not found in the SDK headers");
}
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions tests/deterministic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ fn verify_deterministic() {
&pkg_manifest,
xwin::Arch::X86_64 as u32,
xwin::Variant::Desktop as u32,
false,
true,
)
.unwrap();

Expand Down Expand Up @@ -150,6 +150,6 @@ fn verify_deterministic() {
let expected =
std::fs::read_to_string("tests/expected.txt").expect("failed to read expected.txt");

similar_asserts::assert_eq!(actual, expected);
similar_asserts::assert_eq!(expected, actual);
}
}
Loading

0 comments on commit 685924a

Please sign in to comment.