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.

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

protobuf: improve discovery of protoc executable in build context #20089

Merged
merged 5 commits into from
Dec 12, 2023

Conversation

SpaceIm
Copy link
Contributor

@SpaceIm SpaceIm commented Sep 23, 2023

While building googleapis shared against protobuf shared, and protobuf static in build requirements, I've realized that the protoc flavor linked to shared protobuf was used, instead of the static flavor.

So this PR fixes this issue, by ensuring that even for native build, protoc from build requirements is used when available, instead of protoc from requirements.


@github-actions
Copy link
Contributor

🤖 Beep Boop! This pull request is making changes to 'recipes/protobuf//'.

👋 @Hopobcn you might be interested. 😉

@ghost
Copy link

ghost commented Sep 23, 2023

I detected other pull requests that are modifying protobuf/all recipe:

This message is automatically generated by https://github.com/ericLemanissier/conan-center-conflicting-prs so don't hesitate to report issues/improvements there.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@SpaceIm
Copy link
Contributor Author

SpaceIm commented Oct 7, 2023

ERROR: Missing binary: protobuf/3.20.0:eb5173dacad2e669146d68b06c433f666807fac6

protobuf/3.20.0: WARN: Can't find a 'protobuf/3.20.0' package binary 'eb5173dacad2e669146d68b06c433f666807fac6' for the configuration:
[settings]
arch=x86_64
build_type=Release
compiler=msvc
compiler.cppstd=14
compiler.runtime=dynamic
compiler.runtime_type=Release
compiler.version=192
os=Windows
[options]
debug_suffix=True
lite=False
shared=False
with_rtti=True
with_zlib=True
[requires]
zlib/1.3.Z

ERROR: Missing prebuilt package for 'protobuf/3.20.0'
Check the available packages using 'conan list protobuf/3.20.0:* -r=remote'
'conan test' tested packages must exist, and '--build' argument is used only for the 'test_package' dependencies, not for the tested dependencies

More Info at 'https://docs.conan.io/2/knowledge/faq.html#error-missing-prebuilt-package'

@RubenRBS @danimtb @uilianries Is c3i logic broken? There should be a "lock" delaying execution of test package for shared=True flavor of each settings, until shared=False package of the same settings is available (and if shared=True is the default option value in recipe, it should be the opposite), otherwise when tested recipe is in build requirements of test package, we end up with these missing binaries.

For example here c3i complains that eb5173dacad2e669146d68b06c433f666807fac6 package is missing, it's the static flavor of msvc 192 static Release and it has been built (and even tested) successfully as you can see in https://c3i.jfrog.io/c3i/misc-v2/summary.html?json=https://c3i.jfrog.io/c3i/misc-v2/logs/pr/20089/5-windows-msvc/protobuf/3.20.0//summary.json, so why test package of msvc 192 shared Release can't find this package?

A solution for v2 pipeline might be to inject shared value in requirements AND build requirements.. BUT, it may lead to many issues on macOS due to SIP.

@SpaceIm
Copy link
Contributor Author

SpaceIm commented Oct 7, 2023

I guess that it should pass if I trigger c3i again, since package is available now...

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@SpaceIm
Copy link
Contributor Author

SpaceIm commented Oct 12, 2023

Well, it's super tedious. I've opened #20534

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@ghost ghost mentioned this pull request Oct 17, 2023
3 tasks
@conan-center-bot conan-center-bot added Failed Missing dependencies Build failed due missing dependencies in Conan Center labels Oct 29, 2023
@conan-center-bot

This comment has been minimized.

@SpaceIm
Copy link
Contributor Author

SpaceIm commented Nov 3, 2023

It's a bug of v2 pipeline: #19501 (comment)

@conan-center-bot

This comment has been minimized.

@ghost ghost mentioned this pull request Nov 17, 2023
3 tasks
@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot conan-center-bot removed Failed Missing dependencies Build failed due missing dependencies in Conan Center labels Dec 6, 2023
@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot
Copy link
Collaborator

Conan v1 pipeline ✔️

All green in build 19 (97d1c08d6f632ccee7ab5ffa20b5110a04bb2610):

  • protobuf/3.21.9:
    All packages built successfully! (All logs)

  • protobuf/3.19.6:
    All packages built successfully! (All logs)

  • protobuf/3.21.12:
    All packages built successfully! (All logs)

  • protobuf/3.17.1:
    All packages built successfully! (All logs)

  • protobuf/3.20.0:
    All packages built successfully! (All logs)

  • protobuf/3.20.3:
    All packages built successfully! (All logs)

  • protobuf/3.18.3:
    All packages built successfully! (All logs)


Conan v2 pipeline ✔️

Note: Conan v2 builds are now mandatory. Please read our discussion about it.

All green in build 19 (97d1c08d6f632ccee7ab5ffa20b5110a04bb2610):

  • protobuf/3.21.12:
    All packages built successfully! (All logs)

  • protobuf/3.21.9:
    All packages built successfully! (All logs)

  • protobuf/3.21.4:
    All packages built successfully! (All logs)

  • protobuf/3.19.6:
    All packages built successfully! (All logs)

  • protobuf/3.19.4:
    All packages built successfully! (All logs)

  • protobuf/3.20.0:
    All packages built successfully! (All logs)

  • protobuf/3.18.1:
    All packages built successfully! (All logs)

  • protobuf/3.17.1:
    All packages built successfully! (All logs)

@conan-center-bot conan-center-bot merged commit 7b25813 into conan-io:master Dec 12, 2023
58 checks passed
@SpaceIm SpaceIm deleted the fix/protobuf-find-protoc branch December 12, 2023 21:06
@pgeler
Copy link
Contributor

pgeler commented Dec 13, 2023

is there a way to roll back this? as this causing more harm than good #21737

hoyhoy pushed a commit to hoyhoy/conan-center-index that referenced this pull request Dec 18, 2023
…build context

* improve discovery of protoc executable in build context

* addd more comments

* fix test package
valgur pushed a commit to valgur/conan-center-index that referenced this pull request Jan 1, 2024
…build context

* improve discovery of protoc executable in build context

* addd more comments

* fix test package
valgur pushed a commit to valgur/conan-center-index that referenced this pull request Jan 17, 2024
…build context

* improve discovery of protoc executable in build context

* addd more comments

* fix test package

(cherry picked from commit 7b25813)
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.

6 participants