reindeer icon indicating copy to clipboard operation
reindeer copied to clipboard

Workspace Cargo.toml support + overhaul examples

Open cormacrelf opened this issue 11 months ago • 5 comments

Fixes #51 by

  • Running cargo commands in current dir & prefixing vendor with third-party/...
  • Adding --manifest-path flag
  • Adding manifest_path = "..." + third_party_dir = "..." to reindeer.toml, allowing it to live at your workspace root and just point to where the output should go.
    • (This always bugged me about reindeer, the only real way to use it was with a wrapper script. No longer.)
  • Remaining compatible with the old CLI ways, hopefully.
  • Now that you can run on a workspace toml, filter out workspace members from the BUCK file by default; include_workspace_members = false (default false) because you often want to write your own BUCK files for those instead of having them mixed into a huge generated BUCK file.
  • Overhauls the example, now it's 3 examples with much more documentation. Browse the tree at https://github.com/cormacrelf/reindeer/tree/workspace_cargo_toml/examples to see how it looks.

Altogether you get reindeer --manifest-path ./Cargo.toml --third-party-dir subdirectory buckify, and because this is not something I expect people will want to type, the new config parameters get that down to just reindeer buckify. The layout is as as below.

; cat reindeer.toml

manifest_path = "Cargo.toml"
third_party_dir = "third-party/rust"
...

; reindeer buckify
.
├── Cargo.lock
├── Cargo.toml
├── reindeer.toml
└── third-party
    └── rust
        ├── .cargo/...
        ├── BUCK
        └── vendor

Includes a cargo fmt, let me know if this is wrong, maybe there is configuration drift between this repo & others at meta.

cormacrelf avatar Jan 10 '25 14:01 cormacrelf

Also fixes CI, hopefully.

cormacrelf avatar Jan 12 '25 23:01 cormacrelf

Ok, everything works except windows, and I don't have a windows machine to dig any further into that at the moment. I'm using the exact same blake3/fixups.toml as the buck2 repository, but note that this PR upgrades blake3 to the latest version 1.5.5 (it was 0.1.5 here before). And we get

error[E0433]: failed to resolve: could not find `sse2` in the crate root
   --> .\third-party\blake3-1.5.5.crate\src\platform.rs:123:24
    |
123 |                 crate::sse2::compress_in_place(cv, block, block_len, counter, flags)
    |                        ^^^^ could not find `sse2` in the crate root
    |
note: found an item that was configured out
   --> .\third-party\blake3-1.5.5.crate\src\lib.rs:119:5
    |
119 | mod sse2;
    |     ^^^^
note: the item is gated here
   --> .\third-party\blake3-1.5.5.crate\src\lib.rs:117:1
    |
117 | #[cfg(blake3_sse2_rust)]
    | ^^^^^^^^^^^^^^^^^^^^^^^^
note: found an item that was configured out
   --> .\third-party\blake3-1.5.5.crate\src\lib.rs:122:5
    |
122 | mod sse2;
    |     ^^^^
note: the item is gated here
   --> .\third-party\blake3-1.5.5.crate\src\lib.rs:120:1
    |
120 | #[cfg(blake3_sse2_ffi)]
    | ^^^^^^^^^^^^^^^^^^^^^^^

Even though

  • the x86 shared cfgs from the fixups toml (e.g. --cfg=blake3_sse2_ffi) are applied on mac and linux just fine it seems?
  • Certainly it applies the neon cfgs on an M1 mac.
  • Somehow not there on windows?
  • https://github.com/facebookincubator/reindeer/actions/runs/12807517757/job/35708280984?pr=54

cormacrelf avatar Jan 16 '25 11:01 cormacrelf

Edit, figured out what that was and documented it in the reindeer.toml. Mismatch between prelude's platform names and the ones in this file.

cormacrelf avatar Jan 16 '25 11:01 cormacrelf

Pinging maybe @dtolnay again?

  • There's a Meta check failing for a bad file name that can't be imported. Which file(s) and how can I fix?
  • LMK if you want me to split this up, but the overhauled examples do serve as tests.

cormacrelf avatar Jan 27 '25 02:01 cormacrelf

Friendly bump, would love to have this!

Ralith avatar Mar 24 '25 19:03 Ralith

@dtolnay has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot avatar Apr 13 '25 18:04 facebook-github-bot

Rebased to resolve conflict with #58.

dtolnay avatar Apr 14 '25 19:04 dtolnay

@dtolnay has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot avatar Apr 14 '25 19:04 facebook-github-bot

@dtolnay merged this pull request in facebookincubator/reindeer@2f2b9f3df48332d30ed548a6938657849ccf0c2d.

facebook-github-bot avatar Apr 14 '25 20:04 facebook-github-bot

@cormacrelf I had to insert the following, because otherwise this PR made reindeer generate .cargo/config.toml containing absolute paths. We need to be able to check in .cargo/config.toml into the repo and it cannot contain the path where the local user chose to put their checkout. Perhaps you know a better fix.

https://github.com/facebookincubator/reindeer/blob/2f2b9f3df48332d30ed548a6938657849ccf0c2d/src/vendor.rs#L98-L106

dtolnay avatar Apr 14 '25 20:04 dtolnay

Includes a cargo fmt, let me know if this is wrong, maybe there is configuration drift between this repo & others at meta.

We were on an older rustfmt. This is fixed in https://github.com/facebookincubator/reindeer/commit/ec7a19f7692f65184e7a5a2c4796554124771cf9 + https://github.com/facebookincubator/reindeer/commit/f956d3760e1a9305b78d744d1cdb2c0d37b70cac.

dtolnay avatar Apr 15 '25 20:04 dtolnay

@dtolnay Seems about right with the replacement. I think this is due to cargo running without the chdir it had, so probably a replacement or toml_edit is the best you can do.

Thanks for merging. I'm integrating updated reindeer into a codebase now, fingers crossed it all lines up.

cormacrelf avatar Apr 23 '25 05:04 cormacrelf