Rework planning to be based off the resolve tree and not directly pkgs #282

GregBowyer · 2020-11-06T23:00:54Z

There are a range of subtle and irritating bugs that stem from
attempting to work with renames from the package set provided by cargo
metadata.

Fortunately, recent versions of cargo metadata provide a limited version
of the resolve nodes, both as the original node ids and as a newer set
providing both rename and targetting information.

As such we can (hopefully) simplify crate -> dep node resolution, remove
a bunch of subtle aliasing bugs.

Fixes #241, fixes #269, fixes #270, resolves #144, resolves #187

GregBowyer · 2020-11-06T23:01:22Z

@UebelAndre sorry if this stomps on anything you have in progress

UebelAndre · 2020-11-06T23:10:23Z

It undoubtably will 😢. While I do like the changes here, I would greatly appreciate it if I could get #276 through first. I've had to rebase and resolve conflicts a few times already ☹️

If I could just get it reviewed, I can promise to have it updated as fast as I possibly can to make sure it gets closed as soon as possible 😃

GregBowyer · 2020-11-06T23:38:01Z

impl/src/planning/subplanners.rs

@@ -279,6 +267,8 @@ impl<'planner> WorkspaceSubplanner<'planner> {
      }
    }

+    let mut root_ctx = None;


This bit, and its devil children I dont fully like, it would be nice maybe to just grab the root contect without having to discover it through resolving deps.

I could not think of or find a way to do it, its probably ok for now, and maybe a better future approach might be to move to guppy as a dep

Consider including this as a comment inline (I'm sure as heck not going to remember this, and I think it's good guidance worth remembering)

GregBowyer · 2020-11-06T23:40:22Z

impl/src/planning/subplanners.rs

+      .into_iter()
+      .map(|(target, deps)| {
+        let target = target.unwrap().to_string();
+        let platform_targets = get_matching_bazel_triples(&target, &self.settings.targets)?


I noticed in working with this, the targetting functions are a bit scattered and not really that reused.

As such I removed a bunch of the additional functions, smushing things down towards a single function, and in avoiding allocation-o-rama (which we do a bit frequently in cargo-raze) made it return an iterator.

The part for the generation of bazel conditions I moved into the template itself, as it seemed like a lot of code was written just to essentially prepend a string onto the platform targets.

allocation-o-rama? You mean making copies of stuff?

Yeah I'm usually too lazy to bother with reducing copies because (i've come to believe, perhaps erroneously that) it almost never matters.

Yeah, I kinda ignore it in cargo-raze because the tool runs for me a few times a week, but when calling three consecutive functions seems to produce vector and string of each other it gets a bit much ;)

GregBowyer · 2020-11-06T23:43:58Z

impl/src/planning/subplanners.rs


    let all_skipped_deps = self
      .crate_settings
      .iter()
      .flat_map(|pkg| pkg.skipped_deps.iter())
      .collect::<HashSet<_>>();

-    for dep_id in &self.node.dependencies {
+    for dep in &self.node.deps {


This wonderful part of the metadata gives us both renames and targets. Irritatingly its still a little tricky to detect renames fully (we need this to finally deduce the aliases - see is_renamed)

If rust-lang/cargo#7289 gets solved then a lot of the left-over rename detection code can get murdered

GregBowyer · 2020-11-06T23:46:54Z

impl/src/planning/subplanners.rs

-            .push(buildable_dependency.clone());
-        } else {
-          dep_set.build_deps.push(buildable_dependency.clone());
+      // TODO(GregBowyer): Reimplement what cargo does to detect bad renames


The problem manifests as this

[dependencies] bytes_new = { version = "0.3.0", package = "bytes" } bytes_old = { version = "0.3.0", package = "bytes" }

Strictly this is an error. Right now cargo metadata will basically lose both bytes deps from the resolve graph :gruntle: but a cargo build will scream about having the same basic package with multiple names.

I dont have a good solution right now (I dont think we should go back to trying to essentially emulate the resolve graph like before), so I guess a todo :/

Maybe better to reference a github issue here, but I don't feel strongly about this.

+1 to TODO(#cargo-raze-issue): referencing this error.

GregBowyer · 2020-11-06T23:47:44Z

impl/src/planning/subplanners.rs


+  fn is_dep_targetted(&self, target: Option<&str>) -> bool {


NIT: Maybe add a comment in keeping with the other functions?

GregBowyer · 2020-11-07T00:47:23Z

It undoubtably will cry. While I do like the changes here, I would greatly appreciate it if I could get #276 through first. I've had to rebase and resolve conflicts a few times already frowning_face

If I could just get it reviewed, I can promise to have it updated as fast as I possibly can to make sure it gets closed as soon as possible smiley

I can hold on your changes and work with that, I think we get #127 in then #276 and then see how bad that breaks this.

UebelAndre · 2020-11-07T00:50:11Z

Yeah, I can rebase on top of that when that goes in.

UebelAndre · 2020-11-07T13:11:20Z

#276 is rebased on top of #127 now

GregBowyer · 2020-11-12T01:15:29Z

@acmcarther thoughts?

acmcarther · 2020-11-12T01:39:45Z

Was holding off on review because the other PR is in review, but that's taking a while (because I'm picky or something).

I'll review this now.

GregBowyer · 2020-11-12T01:48:27Z

Nah we are all picky ;P

I am sure this one is going to need more work anyhow there are things I cant think of a better way for that I dont like

acmcarther

Not ideal that the formatter changes are co-mingled with the behavior changes. A bit harder to review.

I am so unqualified to review these PRs at this point. I don't remember much about the implementation and anyway it has changed a lot since I wrote significant code here.

Oh well

acmcarther · 2020-11-10T18:35:41Z

impl/src/planning.rs

@@ -36,8 +36,13 @@ use subplanners::WorkspaceSubplanner;
 /** A ready-to-be-rendered build, containing renderable context for each crate. */
 #[derive(Debug)]
 pub struct PlannedBuild {
+  /// The overall context for this workspace


Does "///" have any meaning beyond "//"? Is this like a rustdoc thing?

Rustdoc thing, but I was noticing in the other review most comments in carg-raze are f the form /** comment */ and so I should make the comments that I think (for the consistency)

acmcarther · 2020-11-12T01:43:53Z

impl/src/planning/subplanners.rs

@@ -279,6 +267,8 @@ impl<'planner> WorkspaceSubplanner<'planner> {
      }
    }

+    let mut root_ctx = None;


Consider including this as a comment inline (I'm sure as heck not going to remember this, and I think it's good guidance worth remembering)

acmcarther · 2020-11-12T01:45:26Z

impl/src/planning/subplanners.rs

+      .iter()
+      .filter(|to_alias| to_alias.is_root_dependency && to_alias.lib_target_name.is_some())
+      .flat_map(|to_alias| {
+        let pkg_name = sanitize_ident(&to_alias.pkg_name);


Are these huge lambdas good rust style? I've been griping about them in @UebelAndre's PRs, but maybe I need to get over it.

From what I have seen it typically is providing its not too crazy but this is pretty much at the limit and might need to just be a function directly.

acmcarther · 2020-11-12T01:49:15Z

impl/src/planning/subplanners.rs

+      .into_iter()
+      .map(|(target, deps)| {
+        let target = target.unwrap().to_string();
+        let platform_targets = get_matching_bazel_triples(&target, &self.settings.targets)?


allocation-o-rama? You mean making copies of stuff?

Yeah I'm usually too lazy to bother with reducing copies because (i've come to believe, perhaps erroneously that) it almost never matters.

acmcarther · 2020-11-12T01:51:51Z

impl/src/planning/subplanners.rs

-            .push(buildable_dependency.clone());
-        } else {
-          dep_set.build_deps.push(buildable_dependency.clone());
+      // TODO(GregBowyer): Reimplement what cargo does to detect bad renames


Maybe better to reference a github issue here, but I don't feel strongly about this.

acmcarther · 2020-11-12T02:06:03Z

impl/src/planning/subplanners.rs

+  ///
+  /// Currently cargo-metadata provides rename detection in a few places, we take the names
+  /// from the resolution of a package
+  fn is_renamed(&self, dep_package: &Package) -> bool {


Does it make sense to precompute some of these new properties? I'm lost in the details here, but part of the idea with the crate catalog was to make this type of lookup more efficient.

Humm yeah might work, and might be a good way to make the find the root sillyness go away.

GregBowyer · 2020-11-12T02:12:55Z

Not ideal that the formatter changes are co-mingled with the behavior changes. A bit harder to review.

I am so unqualified to review these PRs at this point. I don't remember much about the implementation and anyway it has changed a lot since I wrote significant code here.

Oh well

I would only bother with the first commit in the PR, I can just pull the second commit (the reformatting) and we can do a round of that after all crazy PRs get merged

UebelAndre · 2020-11-22T06:03:16Z

Crazy PR has been merged. No more crazy PRs from me for the foreseeable future 😅

UebelAndre · 2021-01-03T20:30:43Z

In hindsight, this should have been merged before any crazy PRs 😞 Sorry about pushing for mine over this one.

@GregBowyer do you have bandwidth in the foreseeable future to revive this effort 😅?

edit: Attempted to do the rebase myself and it's become quite hairy. I think the best course of action would be to more or less start from a clean slate. I think if you drop all changes made to tests then the merge isn't so bad. I've since updated how metadata is mocked for tests to make them deterministic and not require internet. That seems to be the biggest source of conflicts.

GregBowyer · 2021-01-14T01:01:08Z

I have time to pick this up again

UebelAndre

A couple of comments and suggestions but otherwise sounds pretty good. I'm going to need to take another day to page some of the nuances here back into my brain and do another pass but still pretty excited for the direction!

impl/src/context.rs

impl/src/util.rs

impl/src/planning.rs

UebelAndre · 2021-04-23T04:25:31Z

impl/src/planning/subplanners.rs

+      .iter()
+      .filter(|to_alias| to_alias.lib_target_name.is_some())
+      .flat_map(|to_alias| {
+        let pkg_name = util::sanitize_ident(&to_alias.pkg_name);


I feel like the sanitize is worth keeping but wanted to share 9d74090 here in case that's relevant. I think sanatize_indent might need to be updated

Oops I thought I factored that in when I rebased so good catch.

impl/src/planning/subplanners.rs

impl/src/util.rs

UebelAndre · 2021-04-23T04:49:18Z

Since migrating most things to Bazel, ensuring the examples are up to date has not been added to CI. Would you be able to run bazel run //tools:examples_raze and commit the results? I suspect a few things might change based these changes.

GregBowyer · 2021-04-23T18:36:11Z

I will pull in the comments (all sane I think) make sure that test failure is not something sinister.

I think I should also take another scan through the bug list to see if there are any other bugs that this solves.

impl/src/util.rs

GregBowyer · 2021-04-23T22:29:33Z

No need it's not going towards the templates so won't serialise

On Fri, Apr 23, 2021, at 3:08 PM, UebelAndre wrote: ***@***.**** commented on this pull request. In impl/src/util.rs <#282 (comment)>: > @@ -49,20 +51,33 @@ static SUPPORTED_PLATFORM_TRIPLES: &[&str] = &[

"x86_64-unknown-freebsd", ]; +#[derive(Debug, PartialEq, Eq, Copy, Clone)]

…

Specifically asking in regards to #415 <#415> but that's likely out of scope for that change and something I'll go update 😄 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#282 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAA6X3YKL6ITTTIRWFU5P5TTKHVVLANCNFSM4TNHIF4A>.

There are a range of subtle and irritating bugs that stem from attempting to work with renames from the package set provided by cargo metadata. Fortunately, recent versions of cargo metadata provide a limited version of the resolve nodes, both as the original node ids and as a newer set providing both rename and targeting information. As such we can (hopefully) simplify crate -> dep node resolution, remove a bunch of subtle aliasing bugs. Fixes google#241, fixes google#269, fixes google#270, resolves google#144, resolves google#187

UebelAndre

Just another small pass. I'll take some time over the weekend to do a thorough review. The thing I want to look into is the test coverage for this. What's your confidence level that there's enough here to prevent a regression? (again, need to look, there might be enough already but I don't have that context paged in my brain right now 😅)

UebelAndre · 2021-04-24T00:39:19Z

impl/src/planning/subplanners.rs

-        if let Some(platform_details) = &self.platform_details {
-          if let Some(settings_target) = &self.settings.target {
-            let platform = Platform::from_str(&target_str)?;
+  fn is_dep_targetted(&self, target: Option<&String>) -> bool {


nit: Can you add a small doc for this?

UebelAndre · 2021-04-24T00:40:28Z

impl/src/planning/subplanners.rs

-
-    Ok((self._produce_deps(&default_deps)?, targeted_set))
-  }
+  fn process_dep(


nit: add a doc here as well

UebelAndre

Ok, a few more nits and one or two questions. But otherwise I think this is looking really good! 😄

UebelAndre · 2021-04-24T19:14:58Z

impl/src/metadata.rs

@@ -438,7 +438,7 @@ impl RazeMetadataFetcher {
          let version = info.req();
          let src_dir = self.fetch_crate_src(cargo_dir.as_ref(), &name, version)?;
          checksums.insert(
-            package_ident(name, version),
+            package_ident(name, &version),


Was there an issue with this before?

UebelAndre · 2021-04-24T19:33:25Z

impl/src/planning/subplanners.rs

@@ -155,7 +128,7 @@ impl<'planner> WorkspaceSubplanner<'planner> {
    &self,
    node: &Node,
    catalog: &CrateCatalog,
-  ) -> Option<Result<CrateContext>> {
+  ) -> Option<Result<(CrateContext, bool)>> {


Is this not information that already lives (or could already live) in CrateContext? Admittedly that struct could use some cleaning but whether or not the crate is a workspace member seems like it makes sense to store in the context.

cargo-raze/impl/src/context.rs

Lines 116 to 153 in ee2effe

#[derive(Debug, Clone, Serialize)]

pub struct CrateContext {

pub pkg_name: String,

pub pkg_version: Version,

pub edition: String,

pub raze_settings: CrateSettings,

pub canonical_additional_build_file: Option<PathBuf>,

pub default_deps: CrateDependencyContext,

pub targeted_deps: Vec<CrateTargetedDepContext>,

pub license: LicenseData,

pub features: Vec<String>,

pub workspace_path_to_crate: String,

pub workspace_member_dependents: Vec<PathBuf>,

pub workspace_member_dev_dependents: Vec<PathBuf>,

pub workspace_member_build_dependents: Vec<PathBuf>,

pub is_workspace_member_dependency: bool,

pub is_binary_dependency: bool,

pub targets: Vec<BuildableTarget>,

pub build_script_target: Option<BuildableTarget>,

pub links: Option<String>,

pub source_details: SourceDetails,

pub sha256: Option<String>,

pub registry_url: String,

// TODO(acmcarther): This is used internally by renderer to know where to put the build file. It

// probably should live somewhere else. Renderer params (separate from context) should live

// somewhere more explicit.

//

// I'm punting on this now because this requires a more serious look at the renderer code.

pub expected_build_path: String,

// The name of the main lib target for this crate (if present).

// Currently only one such lib can exist per crate.

pub lib_target_name: Option<String>,

// This field tracks whether or not the lib target of `lib_target_name`

// is a proc_macro library or not.

pub is_proc_macro: bool,

}

UebelAndre · 2021-04-24T19:36:36Z

impl/src/rendering/templates/partials/targeted_dependencies.template

-{%- for condition in targeted_dep.conditions %}
-            "{{ condition }}",
+{%- for platform_target in targeted_dep.platform_targets %}
+          "@rules_rust//rust/platform:{{platform_target}}",


Suggested change

"@rules_rust//rust/platform:{{platform_target}}",

"@{{ rust_rules_workspace_name }}//rust/platform:{{platform_target}}",

UebelAndre · 2021-04-24T19:37:02Z

impl/src/rendering/templates/partials/build_script.template

-    {%- for condition in targeted_dep.conditions %}
-            "{{ condition }}",
+    {%- for platform_target in targeted_dep.platform_targets %}
+          "@rules_rust//rust/platform:{{platform_target}}",


Suggested change

"@rules_rust//rust/platform:{{platform_target}}",

"@{{ rust_rules_workspace_name }}//rust/platform:{{platform_target}}",

UebelAndre · 2021-04-24T19:37:58Z

impl/src/testing.rs

@@ -225,7 +225,7 @@ pub fn mock_remote_crate<'server>(
    let enc = flate2::write::GzEncoder::new(tar_gz, Compression::default());
    let mut tar = tar::Builder::new(enc);
    tar
-      .append_dir_all(package_ident(name, version), dir.as_ref().join("archive"))
+      .append_dir_all(package_ident(name, &version), dir.as_ref().join("archive"))


Another question about using a reference. Just curious.

UebelAndre · 2021-04-24T23:35:14Z

@acmcarther this is looking near completion. Don't know if you're around to review it but it'd be great to get in. If not, maybe @dfreese can take a look? It'd probably be good to get your eyes on it too just so you're aware of the changes (sorry for the ping otherwise 😅)

dfreese

Haven't had a chance to make a full pass at this, but had one major question, based on past experience.

dfreese · 2021-04-27T05:57:22Z

impl/src/planning.rs

  pub workspace_context: WorkspaceContext,
+  /// The creates to build for


Suggested change

/// The creates to build for

/// The crates to build for

dfreese · 2021-04-27T06:04:02Z

impl/src/context.rs

+// We only want equality for the first member as it can have multiple names pointing at it.
+impl Hash for DependencyAlias {
+  fn hash<H: Hasher>(&self, state: &mut H) {
+    self.target.hash(state);
+  }
+}
+
+impl PartialEq for DependencyAlias {
+  fn eq(&self, other: &Self) -> bool {
+    self.target == other.target
+  }
+}


I've been bitten by some nasty bugs where hash or comparison functions don't cover the all of the data in the struct.

Is there another way to structure this, perhaps by using map from target string to alias (or &DependencyAlias)?

dfreese · 2021-04-27T06:10:03Z

impl/src/planning/subplanners.rs

-            .push(buildable_dependency.clone());
-        } else {
-          dep_set.build_deps.push(buildable_dependency.clone());
+      // TODO(GregBowyer): Reimplement what cargo does to detect bad renames


+1 to TODO(#cargo-raze-issue): referencing this error.

UebelAndre · 2021-05-01T00:50:26Z

@GregBowyer friendly ping. I feel this is so close to going in 😄

UebelAndre · 2021-05-06T00:58:30Z

@GregBowyer @dfreese another friendly ping 😅 Would love to get this merged and am wondering what outstanding action items are. Happy to help move this forward however I can.

UebelAndre · 2021-05-12T16:57:16Z

Friendly ping again. Happy to help however I can to get this in but would love to see this merged 😄

UebelAndre · 2021-05-16T16:48:44Z

impl/src/planning/subplanners.rs

+      .find(|target| target.kind == "lib" || target.kind == "proc-macro")
+      .map(|target| target.name.clone());
+
+    let is_proc_macro = targets.iter().any(|target| target.kind == "proc_macro");


Suggested change

let is_proc_macro = targets.iter().any(|target| target.kind == "proc_macro");

let is_proc_macro = targets.iter().any(|target| target.kind == "proc-macro");

Aren't string literals fun? 😄

UebelAndre · 2021-05-18T17:36:29Z

@dfreese is this not something you could apply the suggestions for and accept as is? I think it's in a good spot.

dfreese · 2021-05-21T02:01:38Z

@dfreese is this not something you could apply the suggestions for and accept as is? I think it's in a good spot.

#282 (comment)

Not having all fields included as a part of a hash/comparison function causes some incredibly hard bugs to figure out, so I'd want to see if there's an alternative solution to that first.

UebelAndre · 2021-05-21T02:13:52Z

@dfreese is this not something you could apply the suggestions for and accept as is? I think it's in a good spot.

#282 (comment)

Not having all fields included as a part of a hash/comparison function causes some incredibly hard bugs to figure out, so I'd want to see if there's an alternative solution to that first.

Is that not something you or I could patch in this PR or in a follow up? I think @GregBowyer has put fourth a huge amount of effort implementing this PR at least 3 times. I'd hate to have this reimplemented a 4th time and there are other changes I think should go in.

This fixes `crates.bzl` never having any crates defined in the `_PROC_MACRO_DEPENDENCIES` map. This fixes a regression from #425 (or #282) and regenerates cargo-raze outputs (which should have been done on the merged commit). This also fixes a regression in how BUILD files were rendered, resulting in buildifier defects. I cherry picked a change I had an another branch which is ahead of `main` and contains the fix. This change includes #393 Finally, this fixes another regression where `extra_aliased_targets` was being ignored.

GregBowyer force-pushed the alias-bugs branch from 435f71c to cbf3327 Compare November 6, 2020 23:24

GregBowyer mentioned this pull request Nov 6, 2020

When no target set, flate2 fails to compile with latest HEAD #241

Closed

GregBowyer force-pushed the alias-bugs branch from cbf3327 to 37d6cac Compare November 6, 2020 23:31

GregBowyer commented Nov 6, 2020

View reviewed changes

GregBowyer force-pushed the alias-bugs branch from 37d6cac to dc97c23 Compare November 6, 2020 23:51

GregBowyer mentioned this pull request Nov 7, 2020

Allow for aliases at the top level #187

Closed

GregBowyer force-pushed the alias-bugs branch from dc97c23 to 40d9daa Compare November 7, 2020 00:45

GregBowyer force-pushed the alias-bugs branch from 40d9daa to 3d5b94e Compare November 7, 2020 01:02

GregBowyer force-pushed the alias-bugs branch from 3d5b94e to ce72b01 Compare November 9, 2020 22:25

acmcarther requested changes Nov 12, 2020

View reviewed changes

UebelAndre mentioned this pull request Nov 22, 2020

multiple rlib candidates for tokio_02 found #297

Open

UebelAndre mentioned this pull request Jan 4, 2021

How to combine features, platform and dependencies ? #326

Open

joell mentioned this pull request Jan 7, 2021

Erroneous aliases for async-global-executor-1.4.2 #269

Closed

UebelAndre mentioned this pull request Jan 15, 2021

fix(subplanners): Match aliased dependencies by name and semver #346

Merged

UebelAndre requested changes Apr 23, 2021

View reviewed changes

GregBowyer force-pushed the alias-bugs branch 2 times, most recently from 8edc09d to 2063fd5 Compare April 23, 2021 20:58

UebelAndre reviewed Apr 23, 2021

View reviewed changes

impl/src/util.rs Show resolved Hide resolved

GregBowyer force-pushed the alias-bugs branch from 2063fd5 to 98a782b Compare April 24, 2021 00:21

UebelAndre requested changes Apr 24, 2021

View reviewed changes

dfreese self-assigned this Apr 26, 2021

dfreese reviewed Apr 27, 2021

View reviewed changes

UebelAndre mentioned this pull request May 7, 2021

Made some structs sortable and deserializable #415

Merged

UebelAndre reviewed May 16, 2021

View reviewed changes

dfreese mentioned this pull request May 18, 2021

Releases bazelbuild/rules_rust#415

Closed

This was referenced Jul 5, 2021

Reimplement what cargo does to detect bad renames #424

Open

Rework planning to use resolve tree and not packages (updated) #425

Merged

GregBowyer closed this Aug 11, 2021

UebelAndre mentioned this pull request Aug 24, 2021

Fixed regression for proc-macro crates #441

Merged

GregBowyer deleted the alias-bugs branch September 28, 2021 15:50

jgao54 mentioned this pull request Mar 16, 2022

Duplicated alias rule in generated BUILD file #474

Open

	#[derive(Debug, Clone, Serialize)]
	pub struct CrateContext {
	pub pkg_name: String,
	pub pkg_version: Version,
	pub edition: String,
	pub raze_settings: CrateSettings,
	pub canonical_additional_build_file: Option<PathBuf>,
	pub default_deps: CrateDependencyContext,
	pub targeted_deps: Vec<CrateTargetedDepContext>,
	pub license: LicenseData,
	pub features: Vec<String>,
	pub workspace_path_to_crate: String,
	pub workspace_member_dependents: Vec<PathBuf>,
	pub workspace_member_dev_dependents: Vec<PathBuf>,
	pub workspace_member_build_dependents: Vec<PathBuf>,
	pub is_workspace_member_dependency: bool,
	pub is_binary_dependency: bool,
	pub targets: Vec<BuildableTarget>,
	pub build_script_target: Option<BuildableTarget>,
	pub links: Option<String>,
	pub source_details: SourceDetails,
	pub sha256: Option<String>,
	pub registry_url: String,

	// TODO(acmcarther): This is used internally by renderer to know where to put the build file. It
	// probably should live somewhere else. Renderer params (separate from context) should live
	// somewhere more explicit.
	//
	// I'm punting on this now because this requires a more serious look at the renderer code.
	pub expected_build_path: String,

	// The name of the main lib target for this crate (if present).
	// Currently only one such lib can exist per crate.
	pub lib_target_name: Option<String>,
	// This field tracks whether or not the lib target of `lib_target_name`
	// is a proc_macro library or not.
	pub is_proc_macro: bool,
	}

	"@rules_rust//rust/platform:{{platform_target}}",
	"@{{ rust_rules_workspace_name }}//rust/platform:{{platform_target}}",

		pub workspace_context: WorkspaceContext,
		/// The creates to build for

	let is_proc_macro = targets.iter().any(\|target\| target.kind == "proc_macro");
	let is_proc_macro = targets.iter().any(\|target\| target.kind == "proc-macro");

Rework planning to be based off the resolve tree and not directly pkgs #282

Rework planning to be based off the resolve tree and not directly pkgs #282

Conversation

GregBowyer commented Nov 6, 2020

GregBowyer commented Nov 6, 2020

UebelAndre commented Nov 6, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

acmcarther Nov 12, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GregBowyer commented Nov 7, 2020

UebelAndre commented Nov 7, 2020

UebelAndre commented Nov 7, 2020

GregBowyer commented Nov 12, 2020

acmcarther commented Nov 12, 2020

GregBowyer commented Nov 12, 2020

acmcarther left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GregBowyer Nov 12, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

acmcarther Nov 12, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GregBowyer commented Nov 12, 2020

UebelAndre commented Nov 22, 2020

UebelAndre commented Jan 3, 2021 • edited Loading

GregBowyer commented Jan 14, 2021

UebelAndre left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

UebelAndre commented Apr 23, 2021

GregBowyer commented Apr 23, 2021

GregBowyer commented Apr 23, 2021 via email

UebelAndre left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

UebelAndre left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

UebelAndre commented Apr 24, 2021

dfreese left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

UebelAndre commented May 1, 2021

UebelAndre commented May 6, 2021

UebelAndre commented May 12, 2021

Choose a reason for hiding this comment

UebelAndre commented May 18, 2021

dfreese commented May 21, 2021

UebelAndre commented May 21, 2021

acmcarther Nov 12, 2020 •

edited

Loading

GregBowyer Nov 12, 2020 •

edited

Loading

acmcarther Nov 12, 2020 •

edited

Loading

UebelAndre commented Jan 3, 2021 •

edited

Loading