Closed regro-cf-autotick-bot closed 1 year ago
conda-forge-webservices[bot]
commented
1 year ago 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[bot]
commented
1 year ago Hi! This is the friendly conda-forge automerge bot!
I considered the following status checks when analyzing this PR:
Thus the PR was not passing and not merged.
coryan
commented
1 year ago I think the 002-* patch is no longer needed. Let me try to fix that.
h-vetinari
commented
1 year ago 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[^1] and regenerating with
git format-patch tags/v2.12.0 --no-signatureI'm happy to do the latter, but even happier if you want to try your hand at this!
[^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[bot]
commented
1 year ago 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
commented
1 year ago 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
commented
1 year ago 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
commented
1 year ago 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
commented
1 year ago 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:
The list is defined here:
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:
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
commented
1 year ago 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
commented
1 year ago 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?
aiplatformis 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
commented
1 year ago 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?
aiplatformis 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 |
https://github.com/googleapis/google-cloud-cpp/pull/11837 | 4199 |
essentialcontacts |
https://github.com/googleapis/google-cloud-cpp/pull/11850 | 4143 |
domains |
https://github.com/googleapis/google-cloud-cpp/pull/11861 | 6860 |
aiplatform |
https://github.com/googleapis/google-cloud-cpp/pull/11852 | 85887 |
networkservices |
https://github.com/googleapis/google-cloud-cpp/pull/11860 | 13177 |
recaptchaenterprise |
https://github.com/googleapis/google-cloud-cpp/pull/11873 | 5810 |
contentwarehouse |
https://github.com/googleapis/google-cloud-cpp/pull/11875 | 15624 |
gkebackup |
https://github.com/googleapis/google-cloud-cpp/pull/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[bot]
commented
1 year ago 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
commented
1 year ago 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
commented
1 year ago So, windows passed now pretty effortlessly -- looks like aiplatform is really big (like ~1.5h in our windows CI big[^1]).
I'm fine with leaving things like that, but would also be happy to disable it on unix as well.
[^1]: or we lucked into a drastically faster agent for that last run
coryan
commented
1 year ago 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?
It is very likely that the current package version for this feedstock is out of date.
Checklist before merging this PR:
license_fileis packagedInformation about this PR:
please add bot automergein the title and merge the resulting PR. This command will add our bot automerge feature to your feedstock.bot-rerunlabel 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 code>@<space/conda-forge-admin, please rerun bot in a PR comment to have theconda-forge-adminadd 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.
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.