Skip to content

Commit

Permalink
fix: use Target::is_msvc instead of cfg!(target_env="msvc") to av…
Browse files Browse the repository at this point in the history 
…oid mistakenly assuming MSVC debug information during cross-compilation.
  • Loading branch information
@WSH032
WSH032 committed Sep 15, 2024
1 parent 64f8824 commit 1a85cb5
Show file tree
Hide file tree
Showing 4 changed files with 162 additions and 93 deletions.
50 changes: 39 additions & 11 deletions src/build_context.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ use crate::build_options::CargoOptions;
use crate::compile::{warn_missing_py_init, CompileTarget};
use crate::module_writer::{
add_data, include_artifact_for_editable_install, write_bin, write_bindings_module,
write_cffi_module, write_python_part, write_uniffi_module, write_wasm_launcher, WheelWriter,
write_cffi_module, write_python_part, write_uniffi_module, write_wasm_launcher, DebugInfoType,
WheelWriter,
};
use crate::project_layout::ProjectLayout;
use crate::python_interpreter::InterpreterKind;
Expand Down Expand Up @@ -688,6 +689,11 @@ impl BuildContext {
)?;
self.add_external_libs(&mut writer, &[&artifact], &[ext_libs])?;

let with_debuginfo = if self.with_debuginfo {
Some(DebugInfoType::build(&self.target)?)
} else {
None
};
write_bindings_module(
&mut writer,
&self.project_layout,
Expand All @@ -697,7 +703,7 @@ impl BuildContext {
&self.target,
self.editable,
self.pyproject_toml.as_ref(),
self.with_debuginfo,
&with_debuginfo,
)
.context("Failed to add the files to the wheel")?;

Expand Down Expand Up @@ -767,6 +773,11 @@ impl BuildContext {
)?;
self.add_external_libs(&mut writer, &[&artifact], &[ext_libs])?;

let with_debuginfo = if self.with_debuginfo {
Some(DebugInfoType::build(&self.target)?)
} else {
None
};
write_bindings_module(
&mut writer,
&self.project_layout,
Expand All @@ -776,7 +787,7 @@ impl BuildContext {
&self.target,
self.editable,
self.pyproject_toml.as_ref(),
self.with_debuginfo,
&with_debuginfo,
)
.context("Failed to add the files to the wheel")?;

Expand Down Expand Up @@ -863,16 +874,18 @@ impl BuildContext {
if self.editable || matches!(self.auditwheel, AuditWheelMode::Skip) {
return Ok(artifact);
}

let with_debuginfo = if self.with_debuginfo {
Some(DebugInfoType::build(&self.target)?)
} else {
None
};
// auditwheel repair will edit the file, so we need to copy it to avoid errors in reruns
let artifact_path = &artifact.path;
let maturin_build = artifact_path.parent().unwrap().join("maturin");
fs::create_dir_all(&maturin_build)?;
let new_artifact_path = maturin_build.join(artifact_path.file_name().unwrap());
include_artifact_for_editable_install(
artifact_path,
&new_artifact_path,
self.with_debuginfo,
)?;
include_artifact_for_editable_install(artifact_path, &new_artifact_path, &with_debuginfo)?;
artifact.path = new_artifact_path;
Ok(artifact)
}
Expand All @@ -894,6 +907,11 @@ impl BuildContext {
)?;
self.add_external_libs(&mut writer, &[&artifact], &[ext_libs])?;

let with_debuginfo = if self.with_debuginfo {
Some(DebugInfoType::build(&self.target)?)
} else {
None
};
write_cffi_module(
&mut writer,
&self.project_layout,
Expand All @@ -904,7 +922,7 @@ impl BuildContext {
&self.interpreter[0].executable,
self.editable,
self.pyproject_toml.as_ref(),
self.with_debuginfo,
&with_debuginfo,
)?;

self.add_pth(&mut writer)?;
Expand Down Expand Up @@ -961,6 +979,11 @@ impl BuildContext {
)?;
self.add_external_libs(&mut writer, &[&artifact], &[ext_libs])?;

let with_debuginfo = if self.with_debuginfo {
Some(DebugInfoType::build(&self.target)?)
} else {
None
};
write_uniffi_module(
&mut writer,
&self.project_layout,
Expand All @@ -971,7 +994,7 @@ impl BuildContext {
self.target.target_os(),
self.editable,
self.pyproject_toml.as_ref(),
self.with_debuginfo,
&with_debuginfo,
)?;

self.add_pth(&mut writer)?;
Expand Down Expand Up @@ -1069,6 +1092,11 @@ impl BuildContext {
.context("Failed to add the python module to the package")?;
}

let with_debuginfo = if self.with_debuginfo {
Some(DebugInfoType::build(&self.target)?)
} else {
None
};
let mut artifacts_ref = Vec::with_capacity(artifacts.len());
for (artifact, bin_name) in &artifacts_and_files {
artifacts_ref.push(*artifact);
Expand All @@ -1077,7 +1105,7 @@ impl BuildContext {
&artifact.path,
&self.metadata23,
bin_name,
self.with_debuginfo,
&with_debuginfo,
)?;
if self.target.is_wasi() {
write_wasm_launcher(&mut writer, &self.metadata23, bin_name)?;
Expand Down
190 changes: 113 additions & 77 deletions src/module_writer.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -791,86 +791,114 @@ fn handle_cffi_call_result(
}
}

struct ArtifactDebuginfoPaths {
pub(crate) struct MsvcDebugInfo;

struct MsvcArtifactDebuginfoPaths {
/// The target path where debuginfo is to be written
pub(self) target_path: PathBuf,
/// The source path where debuginfo is currently located
pub(self) source_path: PathBuf,
}

fn get_msvc_artifact_debuginfo_path_to_write(
artifact_target_path: &Path,
artifact_source_path: &Path,
) -> Result<ArtifactDebuginfoPaths> {
const DEBUGINFO_EXTENSION: &str = "pdb";

if !cfg!(target_env = "msvc") {
bail!("`--with-debuginfo` is only supported on `MSVC` targets")
impl MsvcDebugInfo {
/// `artifact_target_path` is the target path of the artifact to be written.
/// `artifact_source_path` is the source path where the artifact is located.
pub(self) fn get_msvc_artifact_debuginfo_path_to_write(
&self,
artifact_target_path: &Path,
artifact_source_path: &Path,
) -> Result<MsvcArtifactDebuginfoPaths> {
const DEBUGINFO_EXTENSION: &str = "pdb";

let artifact_name: &Path = artifact_source_path
.file_name()
.ok_or(anyhow!(
"Failed to get file name of {:?}",
artifact_source_path
))?
.as_ref();
let artifact_debuginfo_name: PathBuf = artifact_name
.with_extension(DEBUGINFO_EXTENSION)
.into_os_string()
.into_string()
.map_err(|os_string| {
anyhow!(
"Failed to convert artifact name to utf8 string: {:?}",
os_string
)
})?
// For cdylib, library target names cannot contain hyphens, i.e. `foo-bar.dll` is not allowed.
// But for `bin`, it's allowed, e.g. `foo-bar.exe`. However, the `pbd` file is `foo_bar.pdb`.
// See: <https://github.com/rust-lang/cargo/issues/8519>
//
// We don't need to worry about conflicts between `.pdb` files for `bin` and `cdylib` with the same name,
// because Cargo will issue a warning, and it may be upgraded to an error in the future.
// See: <https://github.com/rust-lang/cargo/issues/6313>
.replace("-", "_")
.into();

let artifact_target_dir = artifact_target_path.parent().ok_or(anyhow!(
"Failed to get parent directory of {:?}",
artifact_target_path
))?;
// On msvc, rustc emits the linker flags `/DEBUG` and `/PDBALTPATH:%_PDB%`,
// so the `.pdb` file should keep the same name as the artifact.
// For example, if the artifact is `foo.dll`, the target path is `/mylib/foo.cp310-win_amd64.pyd`,
// then the debuginfo target path should be `/mylib/foo.pdb`(i.e. not `/mylib/foo.cp310-win_amd64.pdb`).
let artifact_debuginfo_target_path = artifact_target_dir.join(&artifact_debuginfo_name);

let artifact_debuginfo_source_path =
artifact_source_path.with_file_name(artifact_debuginfo_name);

Ok(MsvcArtifactDebuginfoPaths {
target_path: artifact_debuginfo_target_path,
source_path: artifact_debuginfo_source_path,
})
}
}

let artifact_name: &Path = artifact_source_path
.file_name()
.ok_or(anyhow!(
"Failed to get file name of {:?}",
artifact_source_path
))?
.as_ref();
let artifact_debuginfo_name: PathBuf = artifact_name
.with_extension(DEBUGINFO_EXTENSION)
.into_os_string()
.into_string()
.map_err(|os_string| {
anyhow!(
"Failed to convert artifact name to utf8 string: {:?}",
os_string
)
})?
// For cdylib, library target names cannot contain hyphens, i.e. `foo-bar.dll` is not allowed.
// But for `bin`, it's allowed, e.g. `foo-bar.exe`. However, the `pbd` file is `foo_bar.pdb`.
// See: <https://github.com/rust-lang/cargo/issues/8519>
//
// We don't need to worry about conflicts between `.pdb` files for `bin` and `cdylib` with the same name,
// because Cargo will issue a warning, and it may be upgraded to an error in the future.
// See: <https://github.com/rust-lang/cargo/issues/6313>
.replace("-", "_")
.into();

let artifact_target_dir = artifact_target_path.parent().ok_or(anyhow!(
"Failed to get parent directory of {:?}",
artifact_target_path
))?;
// On msvc, rustc emits the linker flags `/DEBUG` and `/PDBALTPATH:%_PDB%`,
// so the `.pdb` file should keep the same name as the artifact.
// For example, if the artifact is `foo.dll`, the target path is `/mylib/foo.cp310-win_amd64.pyd`,
// then the debuginfo target path should be `/mylib/foo.pdb`(i.e. not `/mylib/foo.cp310-win_amd64.pdb`).
let artifact_debuginfo_target_path = artifact_target_dir.join(&artifact_debuginfo_name);

let artifact_debuginfo_source_path =
artifact_source_path.with_file_name(artifact_debuginfo_name);

Ok(ArtifactDebuginfoPaths {
target_path: artifact_debuginfo_target_path,
source_path: artifact_debuginfo_source_path,
})
#[non_exhaustive]
pub(crate) enum DebugInfoType {
Msvc(MsvcDebugInfo),
}

/// Currently, `with_debuginfo` is only supported on `MSVC` targets.
impl DebugInfoType {
pub(crate) fn build(target: &Target) -> Result<Self> {
if target.is_msvc() {
Ok(DebugInfoType::Msvc(MsvcDebugInfo))
} else {
bail!("`--with-debuginfo` is only supported on `MSVC` targets")
}
}
}

/// Writes the artifact to the module writer.
///
/// - `target_path` is the target path to the writer where the artifact is to be written.
/// - `source_path` is the source path where the artifact is currently located.
/// - `with_debuginfo` is the debuginfo type of the artifact. If `Some`, the debuginfo will also be written.
fn write_artifact_to_module_writer(
writer: &mut impl ModuleWriter,
target_path: &Path,
source_path: &Path,
with_debuginfo: bool,
with_debuginfo: &Option<DebugInfoType>,
) -> Result<()> {
// We can't use add_file since we need to mark the file as executable
writer.add_file_with_permissions(target_path, source_path, 0o755)?;

if with_debuginfo {
let ArtifactDebuginfoPaths {
target_path: debuginfo_target_path,
source_path: debuginfo_source_path,
} = get_msvc_artifact_debuginfo_path_to_write(target_path, source_path)?;
if let Some(debuginfo_type) = with_debuginfo {
match debuginfo_type {
DebugInfoType::Msvc(msvc) => {
let MsvcArtifactDebuginfoPaths {
target_path: debuginfo_target_path,
source_path: debuginfo_source_path,
} = msvc.get_msvc_artifact_debuginfo_path_to_write(target_path, source_path)?;

// `.pdb` file doesn't need to be executable, so `0o644` is enough.
writer.add_file(debuginfo_target_path, debuginfo_source_path)?;
}
// `.pdb` file doesn't need to be executable, so `0o644` is enough.
writer.add_file(debuginfo_target_path, debuginfo_source_path)?;
}
}
};
Ok(())
}

Expand All @@ -889,21 +917,29 @@ fn copy_artifact(artifact: &Path, target: &Path) -> Result<()> {
Ok(())
}

/// Currently, `with_debuginfo` is only supported on `MSVC` targets.
/// Includes the artifact for editable install.
///
/// - `artifact` is the artifact to be included.
/// - `target` is the target path where the artifact is to be written.
/// - `with_debuginfo` is the debuginfo type of the artifact. If `Some`, the debuginfo will also be included.
pub(crate) fn include_artifact_for_editable_install(
artifact: &Path,
target: &Path,
with_debuginfo: bool,
with_debuginfo: &Option<DebugInfoType>,
) -> Result<()> {
copy_artifact(artifact, target)?;
if with_debuginfo {
let ArtifactDebuginfoPaths {
target_path: debuginfo_target_path,
source_path: debuginfo_source_path,
} = get_msvc_artifact_debuginfo_path_to_write(target, artifact)?;

copy_artifact(&debuginfo_source_path, &debuginfo_target_path)?;
}
if let Some(debuginfo_type) = with_debuginfo {
match debuginfo_type {
DebugInfoType::Msvc(msvc) => {
let MsvcArtifactDebuginfoPaths {
target_path: debuginfo_target_path,
source_path: debuginfo_source_path,
} = msvc.get_msvc_artifact_debuginfo_path_to_write(target, artifact)?;

copy_artifact(&debuginfo_source_path, &debuginfo_target_path)?;
}
}
};
Ok(())
}

Expand All @@ -919,7 +955,7 @@ pub fn write_bindings_module(
target: &Target,
editable: bool,
pyproject_toml: Option<&PyProjectToml>,
with_debuginfo: bool,
with_debuginfo: &Option<DebugInfoType>,
) -> Result<()> {
let ext_name = &project_layout.extension_name;
let so_filename = if is_abi3 {
Expand Down Expand Up @@ -1005,7 +1041,7 @@ pub fn write_cffi_module(
python: &Path,
editable: bool,
pyproject_toml: Option<&PyProjectToml>,
with_debuginfo: bool,
with_debuginfo: &Option<DebugInfoType>,
) -> Result<()> {
let cffi_declarations = generate_cffi_declarations(crate_dir, target_dir, python)?;

Expand Down Expand Up @@ -1245,7 +1281,7 @@ pub fn write_uniffi_module(
target_os: Os,
editable: bool,
pyproject_toml: Option<&PyProjectToml>,
with_debuginfo: bool,
with_debuginfo: &Option<DebugInfoType>,
) -> Result<()> {
let UniFfiBindings {
name: binding_name,
Expand Down Expand Up @@ -1318,7 +1354,7 @@ pub fn write_bin(
artifact: &Path,
metadata: &Metadata23,
bin_name: &str,
with_debuginfo: bool,
with_debuginfo: &Option<DebugInfoType>,
) -> Result<()> {
let data_dir = PathBuf::from(format!(
"{}-{}.data",
Expand Down
Loading

0 comments on commit 1a85cb5

Please sign in to comment.