Skip to content

Adds set to import flags generation so each path is only added once#6891

Closed
nickgeorge wants to merge 1 commit into
protocolbuffers:masterfrom
nickgeorge:master
Closed

Adds set to import flags generation so each path is only added once#6891
nickgeorge wants to merge 1 commit into
protocolbuffers:masterfrom
nickgeorge:master

Conversation

@nickgeorge
Copy link
Copy Markdown

@nickgeorge nickgeorge commented Nov 14, 2019

Note: this is currently blocking the release of github.com/google/fhir on MacOS, which we are trying to get out for the major FHIR conference of the year starting next week

Currently, if many protos are included from the same location, the paths will be added once per import, which is redundant. In extreme cases, such as https://github.com/google/fhir/blob/master/proto/r4/core/resources/bundle_and_contained_resource.proto , this causes the generation to fail entirely on certain platforms, including MacOS

The pathological case is a proto_library X which depends on a large
number of proto_libraries Y1...Yn, each of which depends on a common
set of proto_libraries Z1...Zm. Without deduplication, import paths for
Z1...Zm will be repeated n times.

@googlebot
Copy link
Copy Markdown

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@nickgeorge nickgeorge changed the title Adds set to import flags generation so each path is only added once cleanup: Adds set to import flags generation so each path is only added once Nov 14, 2019
@rafi-kamal
Copy link
Copy Markdown
Contributor

Hi Nick! Thanks for the PR, but we actually stopped accepting PRs to master branch due to a code yellow until our next release (possibly next week). I understand that you need it to get done as soon as possible for the launch, but Is it possible to depend on the local branch for now?

@rafi-kamal rafi-kamal requested a review from haberman November 14, 2019 23:47
@rafi-kamal
Copy link
Copy Markdown
Contributor

@dankurka since we don't pull in protobuf.bzl to Google3, will it be okay to make an exception here and merge it before the CY ends?

@nickgeorge nickgeorge changed the title cleanup: Adds set to import flags generation so each path is only added once Adds set to import flags generation so each path is only added once Nov 15, 2019
@googlebot
Copy link
Copy Markdown

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@acozzette
Copy link
Copy Markdown

@nickgeorge Thank you for the pull request, but I believe this problem ended up being fixed in #7458.

@acozzette acozzette closed this Oct 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants