Auto merge of #13666 - epage:refactor-package, r=weihanglo

refactor(package): Simplify getting of published Manifest

### What does this PR try to resolve?

This is a parallel effort to #13664 in an effort to #13456

This abstracts away the logic for getting the published version of `Cargo.toml` so we can more easily change the APIs that were previously exposed

### How should we test and review this PR?

### Additional information

Author: bors

src/cargo/core/manifest.rs

@@ -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),
@@ -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

src/cargo/core/package.rs

@@ -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.
@@ -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())

src/cargo/ops/cargo_package.rs

@@ -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 _;
@@ -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,
@@ -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 {
@@ -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());

src/cargo/ops/vendor.rs

@@ -1,4 +1,3 @@
-use crate::core::package::MANIFEST_PREAMBLE;
 use crate::core::shell::Verbosity;
 use crate::core::{GitReference, Package, Workspace};
 use crate::ops;
@@ -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,

src/cargo/util/toml/mod.rs

@@ -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};
@@ -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()))?;
@@ -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> {
+    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,

tests/testsuite/artifact_dep.rs

@@ -2184,7 +2184,7 @@ artifact = [
     "staticlib",
 ]
 target = "target""#,
-                cargo::core::package::MANIFEST_PREAMBLE
+                cargo::core::manifest::MANIFEST_PREAMBLE
             ),
         )],
     );

tests/testsuite/features2.rs

@@ -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();

tests/testsuite/features_namespaced.rs

@@ -1000,7 +1000,7 @@ optional = true
 [features]
 feat = ["opt-dep1"]
 "#,
-                cargo::core::package::MANIFEST_PREAMBLE
+                cargo::core::manifest::MANIFEST_PREAMBLE
             ),
         )],
     );
@@ -1116,7 +1116,7 @@ feat1 = []
 feat2 = ["dep:bar"]
 feat3 = ["feat2"]
 "#,
-                cargo::core::package::MANIFEST_PREAMBLE
+                cargo::core::manifest::MANIFEST_PREAMBLE
             ),
         )],
     );

tests/testsuite/inheritable_workspace_fields.rs

@@ -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
             ),
         )],
     );
@@ -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
             ),
         )],
     );
@@ -532,7 +532,7 @@ authors = []
 version = "0.1.2"
 features = ["testing"]
 "#,
-                cargo::core::package::MANIFEST_PREAMBLE
+                cargo::core::manifest::MANIFEST_PREAMBLE
             ),
         )],
     );
@@ -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
             ),
         )],
     );
@@ -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
             ),
         )],
     );

tests/testsuite/package.rs

@@ -1237,7 +1237,7 @@ registry-index = "{}"
 [dependencies.ghi]
 version = "1.0"
 "#,
-        cargo::core::package::MANIFEST_PREAMBLE,
+        cargo::core::manifest::MANIFEST_PREAMBLE,
         registry.index_url()
     );
 
@@ -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,
@@ -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);
@@ -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);
@@ -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,
@@ -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,
@@ -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.
@@ -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.
@@ -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.

tests/testsuite/publish.rs

@@ -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
                 ),
             ),
             (
@@ -1680,7 +1680,7 @@ repository = "foo"
 
 [dev-dependencies]
 "#,
-                cargo::core::package::MANIFEST_PREAMBLE
+                cargo::core::manifest::MANIFEST_PREAMBLE
             ),
         )],
     );
@@ -1979,7 +1979,7 @@ features = ["cat"]
 version = "1.0"
 features = ["cat"]
 "#,
-                cargo::core::package::MANIFEST_PREAMBLE
+                cargo::core::manifest::MANIFEST_PREAMBLE
             ),
         )],
     );

tests/testsuite/weak_dep_features.rs

@@ -642,7 +642,7 @@ optional = true
 feat1 = []
 feat2 = ["bar?/feat"]
 "#,
-                cargo::core::package::MANIFEST_PREAMBLE
+                cargo::core::manifest::MANIFEST_PREAMBLE
             ),
         )],
     );