Skip to content

google-cloud-cpp v2.12.0#140

Merged
h-vetinari merged 5 commits intoconda-forge:mainfrom
regro-cf-autotick-bot:2.12.0_h610679
Jun 20, 2023
Merged

google-cloud-cpp v2.12.0#140
h-vetinari merged 5 commits intoconda-forge:mainfrom
regro-cf-autotick-bot:2.12.0_h610679

Conversation

@regro-cf-autotick-bot
Copy link
Contributor

It is very likely that the current package version for this feedstock is out of date.

Checklist before merging this PR:

  • Dependencies have been updated if changed: see upstream
  • Tests have passed
  • Updated license if changed and license_file is packaged

Information about this PR:

  1. Feel free to push to the bot's branch to update this PR if needed.
  2. The bot will almost always only open one PR per version.
  3. The bot will stop issuing PRs if more than 3 version bump PRs generated by the bot are open. If you don't want to package a particular version please close the PR.
  4. If you want these PRs to be merged automatically, make an issue with @conda-forge-admin,please add bot automerge in the title and merge the resulting PR. This command will add our bot automerge feature to your feedstock.
  5. If this PR was opened in error or needs to be updated please add the bot-rerun label to this PR. The bot will close this PR and schedule another one. If you do not have permissions to add this label, you can use the phrase @conda-forge-admin, please rerun bot in a PR comment to have the conda-forge-admin add it for you.

Pending Dependency Version Updates

Here is a list of all the pending dependency version updates for this repo. Please double check all dependencies before merging.

Name Upstream Version Current Version
google-cloud-cpp 2.12.0 Anaconda-Server Badge

Dependency Analysis

We couldn't run dependency analysis due to an internal error in the bot. :/ Help is very welcome!

This PR was created by the regro-cf-autotick-bot. The regro-cf-autotick-bot is a service to automatically track the dependency graph, migrate packages, and propose package version updates for conda-forge. Feel free to drop us a line if there are any issues! This PR was generated by https://github.com/regro/cf-scripts/actions/runs/5283800177, please use this URL for debugging.

@conda-forge-webservices
Copy link
Contributor

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@github-actions
Copy link
Contributor

Hi! This is the friendly conda-forge automerge bot!

I considered the following status checks when analyzing this PR:

  • linter: passed
  • azure: failed

Thus the PR was not passing and not merged.

@coryan
Copy link
Contributor

coryan commented Jun 15, 2023

I think the 002-* patch is no longer needed. Let me try to fix that.

@h-vetinari
Copy link
Member

h-vetinari commented Jun 15, 2023

I think the 002-* patch is no longer needed. Let me try to fix that.

Quick-fix is just to comment out that line in the patches: section. Proper fix is rebasing the patches1 and regenerating with

git format-patch tags/v2.12.0 --no-signature

I'm happy to do the latter, but even happier if you want to try your hand at this!

Footnotes

  1. where a feedstock needs patching, I generally have a conda branch in my clone of the upstream sources, and I just need to rebase that when a new version drops. That makes it very easy to handle, especially for such a case as this one, where backported commits will just be auto-skipped by git rebase -i <tag>.

@github-actions
Copy link
Contributor

Hi! This is the friendly conda-forge automerge bot!

It appears that not all commits to this PR were made by the bot. Thus this PR is not being automatically merged. Please add the automerge label again (or ask a maintainer to do so) if you'd like to enable automerge again!

@coryan
Copy link
Contributor

coryan commented Jun 15, 2023

I think the 002-* patch is no longer needed. Let me try to fix that.

Quick-fix is just to comment out that line in the patches: section. Proper fix is rebasing the patches

Thanks. I would have missed rebasing the 0001-* patch, and the line numbers do change.

@h-vetinari
Copy link
Member

Thanks. I would have missed rebasing the 0001-* patch, and the line numbers do change.

The changed line-numbers aren't critical as the patch will still apply with some fuzz, but git generally resolves a lot more things (through its knowledge of the history) than what a dumb application of patch ... will be able to figure out, so it's still good practice to rebase & regenerate.

@h-vetinari h-vetinari changed the title [bot-automerge] google-cloud-cpp v2.12.0 google-cloud-cpp v2.12.0 Jun 16, 2023
Copy link
Member

@h-vetinari h-vetinari left a comment

Choose a reason for hiding this comment

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

LGTM, thanks! :)

@h-vetinari h-vetinari added the automerge Merge the PR when CI passes label Jun 16, 2023
@h-vetinari
Copy link
Member

We're getting into problematic spheres here unfortunately. Windows on protobuf 3.21 (with the disablement patch) already took 5:48h, which is pushing against the 6h hard cutoff enforced by azure. Without that feature disablement, we go from building 6917 build steps to 7027, which is not such a big change, but seemingly enough to push it over the edge (also not all agents are equally spec'd, so there's some randomness involved).

Still, the OSX builds for protobuf 4 are not far behind (way over 5h), so we're probably going to have to start thinking about how to slice up this package somehow. Is there a way to define a core library upon which we can build, or tranches that can be built independently?

@coryan
Copy link
Contributor

coryan commented Jun 16, 2023

Is there a way to define a core library upon which we can build, or tranches that can be built independently?

We should probably move this discussion to a bug? In any case, yes we can do this. The current behavior is to build all the GA libraries:

-DGOOGLE_CLOUD_CPP_ENABLE=__ga_libraries__ \

The list is defined here:

https://github.com/googleapis/google-cloud-cpp/blob/dcc823e8dd34b61fe26aab5bf69f7438ca1801a8/cmake/GoogleCloudCppFeatures.cmake#L36

And it grows over time (maybe one or two features a month, though some "features" can be quite large). There are a set of (implicit) common features that we could build first or separately. Then the features are largely independent from each other, with a small number of caveats:

https://github.com/googleapis/google-cloud-cpp/blob/dcc823e8dd34b61fe26aab5bf69f7438ca1801a8/cmake/GoogleCloudCppFeatures.cmake#L166-L183

I am not sure how to translate this into the conda packaging model. Should we create different packages for the core components and then a package for each feature? That seems a bit overwhelming. Any other thoughts?

@coryan
Copy link
Contributor

coryan commented Jun 16, 2023

While we figure out a more permanent fix, we could disable some of the new features. That would reduce the build time without breaking any existing applications. Thoughts?

aiplatform is particularly large.

@h-vetinari
Copy link
Member

While we figure out a more permanent fix, we could disable some of the new features. That would reduce the build time without breaking any existing applications. Thoughts?

aiplatform is particularly large.

I'm happy to disable features to get us building again. Obvious candidates would be those that got re-enabled in this PR, or those that got added between 2.11 & 2.12. Does the latter apply to aiplatform?

@coryan
Copy link
Contributor

coryan commented Jun 18, 2023

While we figure out a more permanent fix, we could disable some of the new features. That would reduce the build time without breaking any existing applications. Thoughts?
aiplatform is particularly large.

I'm happy to disable features to get us building again. Obvious candidates would be those that got re-enabled in this PR, or those that got added between 2.11 & 2.12. Does the latter apply to aiplatform?

Yes. The new features between 2.11 and 2.12 are:

Name PR Size
timeseriesinsights googleapis/google-cloud-cpp#11837 4199
essentialcontacts googleapis/google-cloud-cpp#11850 4143
domains googleapis/google-cloud-cpp#11861 6860
aiplatform googleapis/google-cloud-cpp#11852 85887
networkservices googleapis/google-cloud-cpp#11860 13177
recaptchaenterprise googleapis/google-cloud-cpp#11873 5810
contentwarehouse googleapis/google-cloud-cpp#11875 15624
gkebackup googleapis/google-cloud-cpp#11875 8922

I used the lines of code as a proxy for "size", it is imperfect, for the well-known reasons (comments, blank lines, documentation, etc.) but good enough for our purposes. As you can see, it is larger than all the other features combined.

I would not be sad if we disabled all the new features until we come up with a better plan. Thoughts?

@github-actions github-actions bot removed the automerge Merge the PR when CI passes label Jun 18, 2023
@github-actions
Copy link
Contributor

Hi! This is the friendly conda-forge automerge bot!

Commits were made to this PR after the automerge label was added. For security reasons, I have disabled automerge by removing the automerge label. Please add the automerge label again (or ask a maintainer to do so) if you'd like to enable automerge again!

@h-vetinari
Copy link
Member

I would not be sad if we disabled all the new features until we come up with a better plan. Thoughts?

For now I'm trying to just disable aiplatform on windows as a sort of "minimal" fix, but I'm not opposed to going as far as disabling all new 2.12 features on all platforms.

@h-vetinari
Copy link
Member

h-vetinari commented Jun 19, 2023

So, windows passed now pretty effortlessly -- looks like aiplatform is really big (like ~1.5h in our windows CI big1).

I'm fine with leaving things like that, but would also be happy to disable it on unix as well.

Footnotes

  1. or we lucked into a drastically faster agent for that last run

@coryan
Copy link
Contributor

coryan commented Jun 19, 2023

I'm fine with leaving things like that, but would also be happy to disable it on unix as well.

It seems like we need to do that too?

@h-vetinari h-vetinari merged commit 30ed83e into conda-forge:main Jun 20, 2023
@regro-cf-autotick-bot regro-cf-autotick-bot deleted the 2.12.0_h610679 branch June 20, 2023 05:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants