feat: create `google-cloud-cpp-core` feedstock #24315

coryan · 2023-10-19T18:01:28Z

google-cloud-cpp is too large to compile in a single feedstock. The CI builds timeout before all the code is compiled.

This PR proposes a new google-cloud-cpp-core feedstock for google-cloud-cpp that packages (1) the common libraries used by all google-cloud-cpp features, and (2) the most popular features of google-cloud-cpp.

These packages are small enough to compile in a single feedstock.

Future PRs will introduce other feedstocks that package more features of google-cloud-cpp.

In these new feedstocks the runtime packages are named libgoogle-cloud-{feat}, and the development packages are named libgoogle-cloud-{feat}-devel.

The libgoogle-cloud / libgoogle-cloud-devel package pair contains the common libraries used by all (or nearly all) subpackages.

There are over 100 libraries in google-cloud-cpp, most of them are small, and most of them see little use. It is not worthwhile to create packages for all these libraries. In a future PR we will introduce the libgoogle-cloud-all and libgoogle-cloud-all-devel packages as catch alls for the smaller and less used features.

When reviewing the checklist below a couple of things to notice:

The package ships static libraries on Windows. It is impossible to ship DLLs at the moment. The existing feedstock ships static libraries. There is ongoing work to enable DLLs.
The maintainers are already in the google-cloud-cpp feedstock. Happy to update the list as needed.

I deviated from the discussion in conda-forge/google-cloud-cpp-feedstock#141 and used libgoogle-cloud-common as the name for the common library. Using libgoogle-cloud looks like would break existing downstream packages and projects.

Part of the work for conda-forge/google-cloud-cpp-feedstock#141

Checklist

`google-cloud-cpp` is too large to compile in a single feedstock. The CI builds timeout before all the code is compiled. This PR proposes a new `google-cloud-cpp-core` feedstock for `google-cloud-cpp` that packages (1) the common libraries used by all `google-cloud-cpp` features, and (2) the most popular features of `google-cloud-cpp`. These packages are small enough to compile in a single feedstock. Future PRs will introduce other feedstocks that package more features of `google-cloud-cpp`. In these new feedstocks the runtime packages are named `libgoogle-cloud-{feat}`, and the development packages are named `libgoogle-cloud-{feat}-devel`. The `libgoogle-cloud` / `libgoogle-cloud-devel` package pair contains the common libraries used by all (or nearly all) subpackages. There are over 100 libraries in `google-cloud-cpp`, most of them are small, and most of them see little use. It is not worthwhile to create packages for all these libraries. In a future PR we will introduce the `libgoogle-cloud-all` and `libgoogle-cloud-all-devel` packages as catch alls for the smaller and less used features.

conda-forge-webservices · 2023-10-19T18:01:48Z

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

I wanted to let you know that I linted all conda-recipes in your PR (recipes/google-cloud-cpp-core) and found some lint.

Here's what I've got...

For recipes/google-cloud-cpp-core:

Selectors are suggested to take a <two spaces>#<one space>[<expression>] form. See lines [30, 79, 197, 278]

conda-forge-webservices · 2023-10-19T19:06:19Z

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 (recipes/google-cloud-cpp-core) and found it was in an excellent condition.

This reverts commit 3111675.

coryan · 2023-10-20T19:51:50Z

@xhochy @h-vetinari

After I made a hilarious number of mistakes with the Windows builds, I think this is ready for review.

I could use some help with the osx build, the errors are in Abseil headers, from code generated by Protobuf. I would have expected this to just work, as both Abseil and Protobuf area already packaged. Any clues?

coryan · 2023-10-20T20:42:13Z

Ahh, looks like this builds with a different SDK:

This fails:
+SDKROOT=/Applications/Xcode_13.2.1.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.9.sdk

https://dev.azure.com/conda-forge/feedstock-builds/_build/results?buildId=808627&view=logs&j=e35d4f76-8ff2-5536-d795-df91e63eb9f7&t=fa7b4b17-b6ff-5c9c-8cfc-15f888c92310&l=253

vs.

This works:
+SDKROOT=/Applications/Xcode_13.2.1.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.13.sdk
https://dev.azure.com/conda-forge/feedstock-builds/_build/results?buildId=799820&view=logs&j=58ac6aab-c4bc-5de2-2894-98e408cc8ec9&t=933f325c-924e-533d-4d95-e93b5843ce8b&l=186

xhochy · 2023-10-21T17:11:30Z

You should be able to select the macOS SDK with a conda_build_config.yaml as done in a feedstock. It should be placed alongside the meta.yaml.

coryan · 2023-10-22T17:55:54Z

Okay, all the tests are passing now. Please take another look.

coryan · 2023-10-25T21:15:32Z

Ping?

h-vetinari

This is a very high-quality PR, thanks a lot @coryan! 🙏

Just a few questions and smaller comments.

W.r.t.

I deviated from the discussion in conda-forge/google-cloud-cpp-feedstock#141 and used libgoogle-cloud-common as the name for the common library. Using libgoogle-cloud looks like would break existing downstream packages and projects.

Can you elaborate what would break? Given the pinning & run-export on google-cloud-cpp, these new packages would not be pulled in without a migration, and such a migration would enable us to validate whether given feedstocks need more than the common libs (there aren't that many consumers anyway).

I don't feel too strongly, but out of the 3 libgoogle-cloud{,-common,-all}, I feel that 2 should suffice.

Though I guess we could have an additional level between the core package and the -all package in the form of "all the outputs of this feedstock together", if that's a grouping that'd be useful (if only to not have to sync the exact list of outputs of this feedstock into google-cloud-cpp).

recipes/google-cloud-cpp-core/meta.yaml

recipes/google-cloud-cpp-core/build.sh

recipes/google-cloud-cpp-core/meta.yaml

coryan · 2023-10-30T14:02:05Z

Ping?

coryan · 2023-11-02T12:35:48Z

@h-vetinari @xhochy any thoughts?

coryan · 2023-11-08T14:37:32Z

Ping?

h-vetinari · 2023-11-12T11:56:56Z

Will try to take a look early next week. Sorry for the wait, busy times, not a lot of FOSS time (and lots of notifications...)

h-vetinari

Basically this looks very good, though a couple of minor points remain.

The one thing that worries me is that test_subpackage("common") apparently succeeded where that should be impossible after the removal of that suffix, which means we'll have to be very careful that the macro-approach works correctly.

recipes/google-cloud-cpp-core/meta.yaml

recipes/google-cloud-cpp-core/bld.bat

recipes/google-cloud-cpp-core/install-libgoogle-cloud.bat

recipes/google-cloud-cpp-core/install-libgoogle-cloud.sh

recipes/google-cloud-cpp-core/meta.yaml

...oogle-cloud-cpp-core/patches/0001-fix-oauth2-only-enable-via-GOOGLE_CLOUD_CPP_ENABLE-1.patch

coryan · 2023-11-27T20:37:48Z

Thanks for the review. Please take another look.

h-vetinari

Aside from some small remaining nits, this LGTM, thanks a lot @coryan! 🥳

Given that this PR is a bit unusual in that:

we're splitting up an existing recipe that times out due to CI constraints (see discussion)
due to the similarity of the subcomponents, a lot of functionality is broken out into macros

... I'd like to ask for some more eyes on this if possible.

PTAL @conda-forge/help-c-cpp

CC @conda-forge/core

recipes/google-cloud-cpp-core/meta.yaml

h-vetinari · 2023-11-28T00:33:52Z

CC @beckermr @wolfv @jakirkham @isuruf about the macro usage

h-vetinari · 2023-12-02T13:55:00Z

Last call for comments, will merge on Tuesday otherwise.

jakirkham · 2023-12-05T03:47:37Z

recipes/google-cloud-cpp-core/conda_build_config.yaml

+MACOSX_DEPLOYMENT_TARGET:
+  - 10.13  # [osx and x86_64]


Seeing issues with feedstock creation for this recipe. Think it has to do with the fact that we are modifying MACOSX_DEPLOYMENT_TARGET, but not restricting that modification to osx and x86_64, which has an adverse affect on other architectures (like osx and arm64). So think we want to add a selector to MACOSX_DEPLOYMENT_TARGET (as shown below)

Suggested change

MACOSX_DEPLOYMENT_TARGET:

- 10.13 # [osx and x86_64]

MACOSX_DEPLOYMENT_TARGET: # [osx and x86_64]

- 10.13 # [osx and x86_64]

Thanks, done in d4fe07d

Not sure what's happening here. The create_feedstocks job still failed, but an empty https://github.com/conda-forge/google-cloud-cpp-core-feedstock got created already.

Co-Authored-By: jakirkham <[email protected]>

jakirkham · 2023-12-05T04:27:52Z

Hmm...looking at a recent feedstock conversion job it still fails. This is happening as part of conda-build attempting to parse the recipe (with some guidance from conda-smithy). It looks like it is having some trouble understand the outputs of the recipe

  File "/home/runner/Miniforge3/lib/python3.10/site-packages/conda_build/metadata.py", line 2146, in extract_single_output_text
    output = output_matches[output_index] if output_matches else ""
IndexError: list index out of range

h-vetinari · 2023-12-05T04:36:38Z

It looks like it is having some trouble understand the outputs of the recipe

This might be related to the use of jinja macros...? Though the CI here was fine, so it's running in some other configuration...

jakirkham · 2023-12-05T04:43:56Z

Not sure. Maybe the thing to do would be to debug this locally?

For whoever might try this, would suggest installing conda-smithy and trying the following

# Clone and `cd` into `staged-recipes` (if not already done)
git clone https://github.com/conda-forge/staged-recipes
cd staged-recipes/recipes

# Use `conda-smithy` to create repo locally
conda smithy init google-cloud-cpp-core
cd google-cloud-cpp-core-feedstock
conda smithy rerender -c auto

That should reproduce the same error

coryan · 2023-12-05T14:32:59Z

It looks like it is having some trouble understand the outputs of the recipe

This might be related to the use of jinja macros...? Though the CI here was fine, so it's running in some other configuration...

It is not the Jinja macros, at least not yet. The function is called with output_name == "google-cloud-cpp-core". It searches for an entry in outputs with name == "google-cloud-cpp-core". There is no such entry because I did not realize one was needed. Should I declare one?

Alternatively, we can rename the feedstock:

git diff
diff --git a/recipes/google-cloud-cpp-core/meta.yaml b/recipes/google-cloud-cpp-core/meta.yaml
index f5e0c9a45d..aa74b0cf72 100644
--- a/recipes/google-cloud-cpp-core/meta.yaml
+++ b/recipes/google-cloud-cpp-core/meta.yaml
@@ -139,7 +139,7 @@ test:
 {%- endmacro %}
 
 package:
-  name: google-cloud-cpp-core
+  name: libgoogle-cloud
   version: {{ version }}
 
 source:

That worked locally, using the conda smithy rerender -c auto repro. Thoughts?

jakirkham · 2023-12-05T20:27:36Z

If you are ok changing the top-level package name, that sounds like a good plan

Think feedstock-name ends up being used for the actual feedstock created. Though not aware of many recipes using this feature. So don't know how well that code path is tested

If this sounds reasonable @coryan , could you please open a PR to make the change?

h-vetinari · 2023-12-05T20:33:55Z

I think what's happening is that we're running into conda/conda-build#5097

h-vetinari · 2023-12-05T20:35:23Z

That worked locally, using the conda smithy rerender -c auto repro. Thoughts?

No, that's not a good idea. In multi-output recipes the recipe-name (under package:) should never match one of the outputs (or you get very counter-intuitive behaviour).

jakirkham · 2023-12-05T21:09:04Z

Hmm...thought the top-level package name was conflicting and we were renaming it not to conflict. Maybe I misunderstood the proposed change

coryan · 2023-12-08T01:59:44Z

That worked locally, using the conda smithy rerender -c auto repro. Thoughts?

No, that's not a good idea. In multi-output recipes the recipe-name (under package:) should never match one of the outputs (or you get very counter-intuitive behaviour).

TIL.

I think what's happening is that we're running into conda/conda-build#5097

The putative fixes for this bug do not seem to work:

~/miniconda3/bin/conda list conda-build

# packages in environment at /usr/local/google/home/coryan/miniconda3:
#
# Name                    Version                   Build  Channel
conda-build               3.28.1          py311h38be061_0    conda-forge

and yet:

conda smithy rerender -c auto

INFO:conda_smithy.configure_feedstock:Downloading conda-forge-pinning-2023.12.08.01.46.42
INFO:conda_smithy.configure_feedstock:Extracting conda-forge-pinning to /usr/local/google/home/coryan/.cache/conda-smithy
INFO:conda_smithy.configure_feedstock:__pycache__ rendering is skipped
INFO:conda_smithy.configure_feedstock:README rendering is skipped
WARNING: Setting build platform. This is only useful when pretending to be on another platform, such as for rendering necessary dependencies on a non-native platform. I trust that you know what you're doing.
WARNING: Setting build arch. This is only useful when pretending to be on another arch, such as for rendering necessary dependencies on a non-native arch. I trust that you know what you're doing.
WARNING: No numpy version specified in conda_build_config.yaml.  Falling back to default numpy value of 1.22
Adding in variants from internal_defaults
Adding in variants from /usr/local/google/home/coryan/.cache/conda-smithy/conda_build_config.yaml
Adding in variants from argument_variants
Adding in variants from /usr/local/google/home/coryan/staged-recipes/recipes/google-cloud-cpp-core-feedstock/recipe/conda_build_config.yaml
Traceback (most recent call last):
  File "/usr/local/google/home/coryan/miniconda3/bin/conda-smithy", line 10, in <module>
    sys.exit(main())
             ^^^^^^
  File "/usr/local/google/home/coryan/miniconda3/lib/python3.11/site-packages/conda_smithy/cli.py", line 725, in main
    args.subcommand_func(args)
  File "/usr/local/google/home/coryan/miniconda3/lib/python3.11/site-packages/conda_smithy/cli.py", line 585, in __call__
    self._call(args, tmpdir)
  File "/usr/local/google/home/coryan/miniconda3/lib/python3.11/site-packages/conda_smithy/cli.py", line 590, in _call
    configure_feedstock.main(
  File "/usr/local/google/home/coryan/miniconda3/lib/python3.11/site-packages/conda_smithy/configure_feedstock.py", line 2552, in main
    render_azure(env, config, forge_dir, return_metadata=True)
  File "/usr/local/google/home/coryan/miniconda3/lib/python3.11/site-packages/conda_smithy/configure_feedstock.py", line 1520, in render_azure
    return _render_ci_provider(
           ^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/google/home/coryan/miniconda3/lib/python3.11/site-packages/conda_smithy/configure_feedstock.py", line 807, in _render_ci_provider
    dump_subspace_config_files(
  File "/usr/local/google/home/coryan/miniconda3/lib/python3.11/site-packages/conda_smithy/configure_feedstock.py", line 479, in dump_subspace_config_files
    configs, top_level_loop_vars = _collapse_subpackage_variants(
                                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/google/home/coryan/miniconda3/lib/python3.11/site-packages/conda_smithy/configure_feedstock.py", line 335, in _collapse_subpackage_variants
    top_level_loop_vars = list_of_metas[0].get_used_loop_vars(
                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/google/home/coryan/miniconda3/lib/python3.11/site-packages/conda_build/metadata.py", line 2620, in get_used_loop_vars
    for var in self.get_used_vars(
               ^^^^^^^^^^^^^^^^^^^
  File "/usr/local/google/home/coryan/miniconda3/lib/python3.11/site-packages/conda_build/metadata.py", line 2723, in get_used_vars
    meta_yaml_reqs = self._get_used_vars_meta_yaml(
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/google/home/coryan/miniconda3/lib/python3.11/site-packages/conda_build/metadata.py", line 2798, in _get_used_vars_meta_yaml
    reqs_text, recipe_text = self._get_used_vars_meta_yaml_helper(
                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/google/home/coryan/miniconda3/lib/python3.11/site-packages/conda_build/metadata.py", line 2781, in _get_used_vars_meta_yaml_helper
    ) + self.extract_single_output_text(
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/google/home/coryan/miniconda3/lib/python3.11/site-packages/conda_build/metadata.py", line 2146, in extract_single_output_text
    output = output_matches[output_index] if output_matches else ""
             ~~~~~~~~~~~~~~^^^^^^^^^^^^^^
IndexError: list index out of range

jakirkham · 2023-12-08T02:09:37Z

recipes/google-cloud-cpp-core/meta.yaml

+package:
+  name: google-cloud-cpp-core
+  version: {{ version }}


Don't think issue ( conda/conda-build#5097 ) applies as package/version is defined

Fair enough. I am a little out of my depth here.

Should I rollback the PR and start over?
Should I manually expand the Jinja macros?
Should I assume conda-build needs a fix and try to figure it out?

Don't think issue ( conda/conda-build#5097 ) applies as package/version is defined

But they're not defined for every output, which is what I understood that issue to be about

The issue says top level

I manually expanded the Jinja macros, that seems to work: #24818

Fix lints

161c3ba

coryan added 13 commits October 19, 2023 20:59

Fix Windows build

38f48c3

Fix test script for Windows

b5f66f1

Also test for version.h on Windows

6c15afb

Another round of fixes for Windows

b369852

Fix for loop (again)

a55bd56

Fix for loop (again)

c082f20

Fix build step on Windows

c0d7646

Fix install step on Windows

a7b0750

Fix variable expansion

bcff933

Fix install for oauth2 on Windows

ba80882

Don't mess with ZLIB just yet

3111675

Revert "Don't mess with ZLIB just yet"

dfe25f2

This reverts commit 3111675.

Include patches to simplify find_package(CURL)

41e27af

coryan marked this pull request as ready for review October 20, 2023 19:49

coryan added 5 commits October 21, 2023 18:08

Address review comments

664f62a

Another attempt at fixing the macOS build

00bd6f5

Hmm... the build says libcxx is needed

67e2b49

Another attempt at fixing macOS

f61f0a5

Apparently libcxx is needed for each subpackage

841aa0a

h-vetinari reviewed Oct 26, 2023

View reviewed changes

Remove stale comments

09426e7

coryan mentioned this pull request Nov 2, 2023

[bot-automerge] google-cloud-cpp v2.18.0 conda-forge/google-cloud-cpp-feedstock#154

Closed

3 tasks

h-vetinari reviewed Nov 22, 2023

View reviewed changes

coryan added 3 commits November 27, 2023 16:17

Address review comments

ceea4eb

Fix library names under Windows

75408ad

I missed some details on Windows

10e865d

h-vetinari approved these changes Nov 28, 2023

View reviewed changes

h-vetinari requested review from xhochy and carterbox November 28, 2023 00:32

Address review comments

a7e1cef

h-vetinari merged commit 6a54192 into conda-forge:main Dec 5, 2023
4 checks passed

jakirkham reviewed Dec 5, 2023

View reviewed changes

jakirkham referenced this pull request Dec 5, 2023

add correct selector to MACOSX_DEPLOYMENT_TARGET

d4fe07d

Co-Authored-By: jakirkham <[email protected]>

coryan deleted the feat-shard-google-cloud-cpp-pr1 branch December 5, 2023 12:46

jakirkham reviewed Dec 8, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: create `google-cloud-cpp-core` feedstock #24315

feat: create `google-cloud-cpp-core` feedstock #24315

coryan commented Oct 19, 2023 •

edited

Loading

conda-forge-webservices bot commented Oct 19, 2023

conda-forge-webservices bot commented Oct 19, 2023

coryan commented Oct 20, 2023

coryan commented Oct 20, 2023

xhochy commented Oct 21, 2023

coryan commented Oct 22, 2023

coryan commented Oct 25, 2023

h-vetinari left a comment

coryan commented Oct 30, 2023

coryan commented Nov 2, 2023

coryan commented Nov 8, 2023

h-vetinari commented Nov 12, 2023

h-vetinari left a comment

coryan commented Nov 27, 2023

h-vetinari left a comment

h-vetinari commented Nov 28, 2023

h-vetinari commented Dec 2, 2023

jakirkham Dec 5, 2023

h-vetinari Dec 5, 2023

h-vetinari Dec 5, 2023

jakirkham commented Dec 5, 2023

h-vetinari commented Dec 5, 2023

jakirkham commented Dec 5, 2023 •

edited

Loading

coryan commented Dec 5, 2023 •

edited

Loading

jakirkham commented Dec 5, 2023

h-vetinari commented Dec 5, 2023

h-vetinari commented Dec 5, 2023

jakirkham commented Dec 5, 2023

coryan commented Dec 8, 2023

jakirkham Dec 8, 2023

coryan Dec 8, 2023

h-vetinari Dec 10, 2023

jakirkham Dec 11, 2023

coryan Dec 18, 2023

feat: create google-cloud-cpp-core feedstock #24315

feat: create google-cloud-cpp-core feedstock #24315

Conversation

coryan commented Oct 19, 2023 • edited Loading

conda-forge-webservices bot commented Oct 19, 2023

conda-forge-webservices bot commented Oct 19, 2023

coryan commented Oct 20, 2023

coryan commented Oct 20, 2023

xhochy commented Oct 21, 2023

coryan commented Oct 22, 2023

coryan commented Oct 25, 2023

h-vetinari left a comment

Choose a reason for hiding this comment

coryan commented Oct 30, 2023

coryan commented Nov 2, 2023

coryan commented Nov 8, 2023

h-vetinari commented Nov 12, 2023

h-vetinari left a comment

Choose a reason for hiding this comment

coryan commented Nov 27, 2023

h-vetinari left a comment

Choose a reason for hiding this comment

h-vetinari commented Nov 28, 2023

h-vetinari commented Dec 2, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakirkham commented Dec 5, 2023

h-vetinari commented Dec 5, 2023

jakirkham commented Dec 5, 2023 • edited Loading

coryan commented Dec 5, 2023 • edited Loading

jakirkham commented Dec 5, 2023

h-vetinari commented Dec 5, 2023

h-vetinari commented Dec 5, 2023

jakirkham commented Dec 5, 2023

coryan commented Dec 8, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

feat: create `google-cloud-cpp-core` feedstock #24315

feat: create `google-cloud-cpp-core` feedstock #24315

coryan commented Oct 19, 2023 •

edited

Loading

jakirkham commented Dec 5, 2023 •

edited

Loading

coryan commented Dec 5, 2023 •

edited

Loading