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.

Sign up for GitHub

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

Jump to bottom

Co-locate generated files #3049

Merged
merged 9 commits into from
Aug 1, 2024
Merged

Co-locate generated files #3049

merged 9 commits into from
Aug 1, 2024

Conversation

brentleyjones
Copy link
Contributor

Resolves #3037.

@brentleyjones brentleyjones force-pushed the bj/inline-bazel-generated-files branch 3 times, most recently from ab82865 to 9ff77e8 Compare July 8, 2024 21:06
@brentleyjones brentleyjones force-pushed the bj/inline-bazel-generated-files branch from 9ff77e8 to 1af26b5 Compare July 9, 2024 13:51
@erikkerber
Copy link
Contributor

This fixes those "top-level" generated folders from our internal upstream project, and looks pretty good.

We have enough configs with sim+device times 3 product variants that there are 6 generated folders per library, so it's not a silver arrow for making things drop-dead-easy for Apple devs not fluent in Bazel, but it's also a big improvement over what we had previously.

@sebastianv1
Copy link
Contributor

sebastianv1 commented Jul 10, 2024 •
edited
Loading

What if we added an option to map a configuration name to something more idiomatic/familiar? ios_arm64-dbg-ios-arm64-min15.0-applebin_ios-ST-a79286d91b1b: iOS 15 Device Build. It's manual & brittle as configurations change over time but is pretty flexible in naming?

@brentleyjones
Copy link
Contributor Author

We might be able to smartly translate those, since we know the platform, minOS, and arch for any given config by virtue of targets that have that config. So these could be named similar to how we do target disambiguation 🤔. That would be a followup PR though.

@sebastianv1 sebastianv1 force-pushed the bj/inline-bazel-generated-files branch from b408d68 to 01bb4d2 Compare July 26, 2024 17:41
@sebastianv1 sebastianv1 force-pushed the bj/inline-bazel-generated-files branch from 787ffb8 to ea062fe Compare July 29, 2024 19:15
@sebastianv1 sebastianv1 force-pushed the bj/inline-bazel-generated-files branch from 573c0ff to 8d2a6c9 Compare July 30, 2024 14:26
@sebastianv1 sebastianv1 force-pushed the bj/inline-bazel-generated-files branch from 71d7dfe to 9fe354a Compare July 30, 2024 18:32
@brentleyjones brentleyjones marked this pull request as ready for review July 31, 2024 13:41
@brentleyjones brentleyjones requested a review from a team as a code owner July 31, 2024 13:41
@brentleyjones
Copy link
Contributor Author

brentleyjones commented Jul 31, 2024 •
edited
Loading

@erikkerber @sebastianv1 Can you approve if works for you locally?

sebastianv1
sebastianv1 approved these changes Jul 31, 2024
View reviewed changes
Copy link
Contributor

@sebastianv1 sebastianv1 left a comment

Choose a reason for hiding this comment

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

Thank you for the help!

@brentleyjones brentleyjones enabled auto-merge (squash) July 31, 2024 22:59
@brentleyjones brentleyjones disabled auto-merge August 1, 2024 14:34
@brentleyjones brentleyjones enabled auto-merge (squash) August 1, 2024 14:34
brentleyjones and others added 6 commits August 1, 2024 09:35
@brentleyjones
Encode “group” or “file” PathTreeNodes into the type system
e08d42c 
This makes it clear which type of node we are dealing with. It also sets us up for future enhancements where we don’t exactly map to the filesystem (e.g. co-locating generated outputs with source files).

Signed-off-by: Brentley Jones <github@brentleyjones.com>
@brentleyjones
Co-locate bazel generated files
330f80e 
Signed-off-by: Brentley Jones <github@brentleyjones.com>
@sebastianv1 @brentleyjones
Fixup write_files_and_groups_tests
eb02028 
Update the API to support the generated_files/folder mocks structure

Signed-off-by: Sebastian Shanus <sebastian.shanus@robinhood.com>
@sebastianv1 @brentleyjones
Fix nondeterministic multipleConfigs sorting
fe97369 
The path tree layout is nondeterministic from its upstream grouping in a dictionary. Putting the sort here seemed best since other path tree sorting happens here.

Reviewers:
Signed-off-by: Sebastian Shanus <sebastian.shanus@robinhood.com>
@sebastianv1 @brentleyjones
Add ElementCreator tests
b5dcaf2 
Adds tests and basic coverage over the new `ElementCreator` types.

Signed-off-by: Sebastian Shanus <sebastian.shanus@robinhood.com>
@sebastianv1 @brentleyjones
Update multipleConfigs sorting
7ae5270 
Relocates up into `calculatePathTree`

Signed-off-by: Sebastian Shanus <sebastian.shanus@robinhood.com>
@sebastianv1 @brentleyjones
Use mock label creation
dbc1a63 
Avoids constructing the native `Label` for a mocked `struct` with the required properties.

Signed-off-by: Sebastian Shanus <sebastian.shanus@robinhood.com>
@sebastianv1 @brentleyjones
Use string_dict to model generated_files/folders
ddbde30 
No need to future proof the model as it stands, we only need the path + owner as a model.

Signed-off-by: Sebastian Shanus <sebastian.shanus@robinhood.com>
@sebastianv1 @brentleyjones
Assert Group Element contents
95bced0 
Adds assertions for group element contents.

Signed-off-by: Sebastian Shanus <sebastian.shanus@robinhood.com>
@brentleyjones brentleyjones force-pushed the bj/inline-bazel-generated-files branch from 9fe354a to 95bced0 Compare August 1, 2024 14:35
@brentleyjones brentleyjones merged commit d9715d4 into main Aug 1, 2024
15 of 16 checks passed
@brentleyjones brentleyjones deleted the bj/inline-bazel-generated-files branch August 1, 2024 14:41
@brentleyjones brentleyjones mentioned this pull request Aug 12, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sebastianv1 sebastianv1 sebastianv1 approved these changes

@erikkerber erikkerber erikkerber approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Feature Request: Co-locate Bazel Generated Files with targets
3 participants
@brentleyjones @erikkerber @sebastianv1