Skip to content
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

refactor(package): Simplify getting of published Manifest #13666

Merged
merged 5 commits into from
Mar 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions src/cargo/core/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,19 @@ use crate::util::errors::*;
use crate::util::interning::InternedString;
use crate::util::{short_hash, Filesystem, GlobalContext};

pub const MANIFEST_PREAMBLE: &str = "\
# THIS FILE IS AUTOMATICALLY GENERATED BY CARGO
#
# When uploading crates to the registry Cargo will automatically
# \"normalize\" Cargo.toml files for maximal compatibility
# with all versions of Cargo and also rewrite `path` dependencies
# to registry (e.g., crates.io) dependencies.
#
# If you are reading this file be aware that the original Cargo.toml
# will likely look very different (and much more reasonable).
# See Cargo.toml.orig for the original contents.
";

pub enum EitherManifest {
Real(Manifest),
Virtual(VirtualManifest),
Expand Down Expand Up @@ -470,6 +483,10 @@ impl Manifest {
pub fn contents(&self) -> &str {
self.contents.as_str()
}
pub fn to_resolved_contents(&self) -> CargoResult<String> {
let toml = toml::to_string_pretty(self.resolved_toml())?;
Ok(format!("{}\n{}", MANIFEST_PREAMBLE, toml))
}
/// Collection of spans for the original TOML
pub fn document(&self) -> &toml_edit::ImDocument<String> {
&self.document
Expand Down
20 changes: 0 additions & 20 deletions src/cargo/core/package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,22 +32,8 @@ use crate::util::network::http::http_handle_and_timeout;
use crate::util::network::http::HttpTimeout;
use crate::util::network::retry::{Retry, RetryResult};
use crate::util::network::sleep::SleepTracker;
use crate::util::toml::prepare_for_publish;
use crate::util::{self, internal, GlobalContext, Progress, ProgressStyle};

pub const MANIFEST_PREAMBLE: &str = "\
# THIS FILE IS AUTOMATICALLY GENERATED BY CARGO
#
# When uploading crates to the registry Cargo will automatically
# \"normalize\" Cargo.toml files for maximal compatibility
# with all versions of Cargo and also rewrite `path` dependencies
# to registry (e.g., crates.io) dependencies.
#
# If you are reading this file be aware that the original Cargo.toml
# will likely look very different (and much more reasonable).
# See Cargo.toml.orig for the original contents.
";

/// Information about a package that is available somewhere in the file system.
///
/// A package is a `Cargo.toml` file plus all the files that are part of it.
Expand Down Expand Up @@ -197,12 +183,6 @@ impl Package {
}
}

pub fn to_registry_toml(&self, ws: &Workspace<'_>) -> CargoResult<String> {
let manifest = prepare_for_publish(self.manifest().resolved_toml(), ws, self.root())?;
let toml = toml::to_string_pretty(&manifest)?;
Ok(format!("{}\n{}", MANIFEST_PREAMBLE, toml))
}

/// Returns if package should include `Cargo.lock`.
pub fn include_lockfile(&self) -> bool {
self.targets().iter().any(|t| t.is_example() || t.is_bin())
Expand Down
32 changes: 6 additions & 26 deletions src/cargo/ops/cargo_package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use crate::sources::PathSource;
use crate::util::cache_lock::CacheLockMode;
use crate::util::context::JobsConfig;
use crate::util::errors::CargoResult;
use crate::util::toml::{prepare_for_publish, to_real_manifest};
use crate::util::toml::prepare_for_publish;
use crate::util::{self, human_readable_bytes, restricted_names, FileLock, GlobalContext};
use crate::{drop_println, ops};
use anyhow::Context as _;
Expand Down Expand Up @@ -448,32 +448,11 @@ fn error_custom_build_file_not_in_package(
}

/// Construct `Cargo.lock` for the package to be published.
fn build_lock(ws: &Workspace<'_>, orig_pkg: &Package) -> CargoResult<String> {
fn build_lock(ws: &Workspace<'_>, publish_pkg: &Package) -> CargoResult<String> {
let gctx = ws.gctx();
let orig_resolve = ops::load_pkg_lockfile(ws)?;

// Convert Package -> TomlManifest -> Manifest -> Package
let contents = orig_pkg.manifest().contents();
let document = orig_pkg.manifest().document();
let toml_manifest =
prepare_for_publish(orig_pkg.manifest().resolved_toml(), ws, orig_pkg.root())?;
let source_id = orig_pkg.package_id().source_id();
let mut warnings = Default::default();
let mut errors = Default::default();
let manifest = to_real_manifest(
contents.to_owned(),
document.clone(),
toml_manifest,
source_id,
orig_pkg.manifest_path(),
gctx,
&mut warnings,
&mut errors,
)?;
let new_pkg = Package::new(manifest, orig_pkg.manifest_path());

// Regenerate Cargo.lock using the old one as a guide.
let tmp_ws = Workspace::ephemeral(new_pkg, ws.gctx(), None, true)?;
let tmp_ws = Workspace::ephemeral(publish_pkg.clone(), ws.gctx(), None, true)?;
let mut tmp_reg = PackageRegistry::new(ws.gctx())?;
let mut new_resolve = ops::resolve_with_previous(
&mut tmp_reg,
Expand Down Expand Up @@ -704,6 +683,7 @@ fn tar(

let base_name = format!("{}-{}", pkg.name(), pkg.version());
let base_path = Path::new(&base_name);
let publish_pkg = prepare_for_publish(pkg, ws)?;

let mut uncompressed_size = 0;
for ar_file in ar_files {
Expand Down Expand Up @@ -734,8 +714,8 @@ fn tar(
}
FileContents::Generated(generated_kind) => {
let contents = match generated_kind {
GeneratedFile::Manifest => pkg.to_registry_toml(ws)?,
GeneratedFile::Lockfile => build_lock(ws, pkg)?,
GeneratedFile::Manifest => publish_pkg.manifest().to_resolved_contents()?,
GeneratedFile::Lockfile => build_lock(ws, &publish_pkg)?,
GeneratedFile::VcsInfo(ref s) => serde_json::to_string_pretty(s)?,
};
header.set_entry_type(EntryType::file());
Expand Down
4 changes: 1 addition & 3 deletions src/cargo/ops/vendor.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use crate::core::package::MANIFEST_PREAMBLE;
use crate::core::shell::Verbosity;
use crate::core::{GitReference, Package, Workspace};
use crate::ops;
Expand Down Expand Up @@ -360,8 +359,7 @@ fn cp_sources(
let cksum = if dst.file_name() == Some(OsStr::new("Cargo.toml"))
&& pkg.package_id().source_id().is_git()
{
let original_toml = toml::to_string_pretty(pkg.manifest().resolved_toml())?;
let contents = format!("{}\n{}", MANIFEST_PREAMBLE, original_toml);
let contents = pkg.manifest().to_resolved_contents()?;
copy_and_checksum(
&dst,
&mut dst_opts,
Expand Down
30 changes: 26 additions & 4 deletions src/cargo/util/toml/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use crate::core::dependency::{Artifact, ArtifactTarget, DepKind};
use crate::core::manifest::{ManifestMetadata, TargetSourcePath};
use crate::core::resolver::ResolveBehavior;
use crate::core::{find_workspace_root, resolve_relative_path, CliUnstable, FeatureValue};
use crate::core::{Dependency, Manifest, PackageId, Summary, Target};
use crate::core::{Dependency, Manifest, Package, PackageId, Summary, Target};
use crate::core::{Edition, EitherManifest, Feature, Features, VirtualManifest, Workspace};
use crate::core::{GitReference, PackageIdSpec, SourceId, WorkspaceConfig, WorkspaceRootConfig};
use crate::sources::{CRATES_IO_INDEX, CRATES_IO_REGISTRY};
Expand All @@ -48,8 +48,8 @@ pub fn read_manifest(
source_id: SourceId,
gctx: &GlobalContext,
) -> CargoResult<EitherManifest> {
let mut warnings = vec![];
let mut errors = vec![];
let mut warnings = Default::default();
let mut errors = Default::default();

let contents =
read_toml_string(path, gctx).map_err(|err| ManifestError::new(err, path.into()))?;
Expand Down Expand Up @@ -218,10 +218,32 @@ fn warn_on_unused(unused: &BTreeSet<String>, warnings: &mut Vec<String>) {
}
}

pub fn prepare_for_publish(me: &Package, ws: &Workspace<'_>) -> CargoResult<Package> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While not necessary, I feel like prepare_for_publish and friends could be moved to a dedicated module.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been going back and forth on it, so I've just left it for now.

The main benefit for keeping it here is to keep awareness of it when adding new features to the manifest in the hope that we remember that they might impact this so we update it.

let contents = me.manifest().contents();
let document = me.manifest().document();
let toml_manifest = prepare_toml_for_publish(me.manifest().resolved_toml(), ws, me.root())?;
let source_id = me.package_id().source_id();
let mut warnings = Default::default();
let mut errors = Default::default();
let gctx = ws.gctx();
let manifest = to_real_manifest(
contents.to_owned(),
document.clone(),
toml_manifest,
source_id,
me.manifest_path(),
gctx,
&mut warnings,
&mut errors,
)?;
let new_pkg = Package::new(manifest, me.manifest_path());
Ok(new_pkg)
}

/// Prepares the manifest for publishing.
// - Path and git components of dependency specifications are removed.
// - License path is updated to point within the package.
pub fn prepare_for_publish(
fn prepare_toml_for_publish(
me: &manifest::TomlManifest,
ws: &Workspace<'_>,
package_root: &Path,
Expand Down
2 changes: 1 addition & 1 deletion tests/testsuite/artifact_dep.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2184,7 +2184,7 @@ artifact = [
"staticlib",
]
target = "target""#,
cargo::core::package::MANIFEST_PREAMBLE
cargo::core::manifest::MANIFEST_PREAMBLE
),
)],
);
Expand Down
2 changes: 1 addition & 1 deletion tests/testsuite/features2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1708,7 +1708,7 @@ homepage = "https://example.com/"
license = "MIT"
resolver = "2"
"#,
cargo::core::package::MANIFEST_PREAMBLE
cargo::core::manifest::MANIFEST_PREAMBLE
);

let f = File::open(&p.root().join("target/package/a-0.1.0.crate")).unwrap();
Expand Down
4 changes: 2 additions & 2 deletions tests/testsuite/features_namespaced.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1000,7 +1000,7 @@ optional = true
[features]
feat = ["opt-dep1"]
"#,
cargo::core::package::MANIFEST_PREAMBLE
cargo::core::manifest::MANIFEST_PREAMBLE
),
)],
);
Expand Down Expand Up @@ -1116,7 +1116,7 @@ feat1 = []
feat2 = ["dep:bar"]
feat3 = ["feat2"]
"#,
cargo::core::package::MANIFEST_PREAMBLE
cargo::core::manifest::MANIFEST_PREAMBLE
),
)],
);
Expand Down
10 changes: 5 additions & 5 deletions tests/testsuite/inheritable_workspace_fields.rs
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ repository = "https://github.com/example/example"
branch = "master"
repository = "https://gitlab.com/rust-lang/rust"
"#,
cargo::core::package::MANIFEST_PREAMBLE
cargo::core::manifest::MANIFEST_PREAMBLE
),
)],
);
Expand Down Expand Up @@ -406,7 +406,7 @@ version = "0.5.2"
[build-dependencies.dep-build]
version = "0.8"
"#,
cargo::core::package::MANIFEST_PREAMBLE
cargo::core::manifest::MANIFEST_PREAMBLE
),
)],
);
Expand Down Expand Up @@ -532,7 +532,7 @@ authors = []
version = "0.1.2"
features = ["testing"]
"#,
cargo::core::package::MANIFEST_PREAMBLE
cargo::core::manifest::MANIFEST_PREAMBLE
),
)],
);
Expand Down Expand Up @@ -796,7 +796,7 @@ repository = "https://github.com/example/example"
branch = "master"
repository = "https://gitlab.com/rust-lang/rust"
"#,
cargo::core::package::MANIFEST_PREAMBLE
cargo::core::manifest::MANIFEST_PREAMBLE
),
)],
);
Expand Down Expand Up @@ -959,7 +959,7 @@ version = "0.5.2"
[build-dependencies.dep-build]
version = "0.8"
"#,
cargo::core::package::MANIFEST_PREAMBLE
cargo::core::manifest::MANIFEST_PREAMBLE
),
)],
);
Expand Down
18 changes: 9 additions & 9 deletions tests/testsuite/package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1237,7 +1237,7 @@ registry-index = "{}"
[dependencies.ghi]
version = "1.0"
"#,
cargo::core::package::MANIFEST_PREAMBLE,
cargo::core::manifest::MANIFEST_PREAMBLE,
registry.index_url()
);

Expand Down Expand Up @@ -1294,7 +1294,7 @@ name = "bar"
version = "0.1.0"
authors = []
"#,
cargo::core::package::MANIFEST_PREAMBLE
cargo::core::manifest::MANIFEST_PREAMBLE
);
validate_crate_contents(
f,
Expand Down Expand Up @@ -1367,7 +1367,7 @@ version = "1.0.0"
[target.{host}.dependencies.baz]
version = "1.0.0"
"#,
cargo::core::package::MANIFEST_PREAMBLE,
cargo::core::manifest::MANIFEST_PREAMBLE,
host = rustc_host()
);
verify(&p, "package", rewritten_toml);
Expand All @@ -1387,7 +1387,7 @@ public = true
version = "1.0.0"
public = true
"#,
cargo::core::package::MANIFEST_PREAMBLE,
cargo::core::manifest::MANIFEST_PREAMBLE,
host = rustc_host()
);
verify(&p, "package -Zpublic-dependency", rewritten_toml);
Expand Down Expand Up @@ -2808,7 +2808,7 @@ name = "bar"
version = "0.1.0"
resolver = "1"
"#,
cargo::core::package::MANIFEST_PREAMBLE
cargo::core::manifest::MANIFEST_PREAMBLE
);
validate_crate_contents(
f,
Expand All @@ -2826,7 +2826,7 @@ edition = "2015"
name = "baz"
version = "0.1.0"
"#,
cargo::core::package::MANIFEST_PREAMBLE
cargo::core::manifest::MANIFEST_PREAMBLE
);
validate_crate_contents(
f,
Expand Down Expand Up @@ -2891,7 +2891,7 @@ description = "foo"
homepage = "https://example.com/"
license = "MIT"
"#,
cargo::core::package::MANIFEST_PREAMBLE
cargo::core::manifest::MANIFEST_PREAMBLE
);
let cargo_lock_contents = r#"# This file is automatically @generated by Cargo.
# It is not intended for manual editing.
Expand Down Expand Up @@ -2985,7 +2985,7 @@ description = "foo"
documentation = "https://example.com/"
license = "MIT"
"#,
cargo::core::package::MANIFEST_PREAMBLE
cargo::core::manifest::MANIFEST_PREAMBLE
);
let cargo_lock_contents = r#"# This file is automatically @generated by Cargo.
# It is not intended for manual editing.
Expand Down Expand Up @@ -3092,7 +3092,7 @@ description = "foo"
homepage = "https://example.com/"
license = "MIT"
"#,
cargo::core::package::MANIFEST_PREAMBLE
cargo::core::manifest::MANIFEST_PREAMBLE
);
let cargo_lock_contents = r#"# This file is automatically @generated by Cargo.
# It is not intended for manual editing.
Expand Down
6 changes: 3 additions & 3 deletions tests/testsuite/publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1566,7 +1566,7 @@ You may press ctrl-c [..]
[dependencies.dep1]\n\
version = \"1.0\"\n\
",
cargo::core::package::MANIFEST_PREAMBLE
cargo::core::manifest::MANIFEST_PREAMBLE
),
),
(
Expand Down Expand Up @@ -1680,7 +1680,7 @@ repository = "foo"

[dev-dependencies]
"#,
cargo::core::package::MANIFEST_PREAMBLE
cargo::core::manifest::MANIFEST_PREAMBLE
),
)],
);
Expand Down Expand Up @@ -1979,7 +1979,7 @@ features = ["cat"]
version = "1.0"
features = ["cat"]
"#,
cargo::core::package::MANIFEST_PREAMBLE
cargo::core::manifest::MANIFEST_PREAMBLE
),
)],
);
Expand Down
2 changes: 1 addition & 1 deletion tests/testsuite/weak_dep_features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -642,7 +642,7 @@ optional = true
feat1 = []
feat2 = ["bar?/feat"]
"#,
cargo::core::package::MANIFEST_PREAMBLE
cargo::core::manifest::MANIFEST_PREAMBLE
),
)],
);
Expand Down