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

arrow: fix self.options and self.settings access in package_id() #19296

Closed
wants to merge 1 commit into from

Conversation

valgur
Copy link
Contributor

@valgur valgur commented Aug 19, 2023

self.options and self.settings can no longer be accessed from package_id() in Conan v2.

I'm very open to suggestions if anybody has a better solution.

@ghost
Copy link

ghost commented Aug 19, 2023

@conan-center-bot
Copy link
Collaborator

Conan v1 pipeline ✔️

All green in build 1 (283ac516c2ef0aee54cc7c40c0058ba5a0289b33):

  • arrow/1.0.0:
    All packages built successfully! (All logs)

  • arrow/10.0.1:
    All packages built successfully! (All logs)

  • arrow/11.0.0:
    All packages built successfully! (All logs)

  • arrow/12.0.0:
    All packages built successfully! (All logs)

  • arrow/2.0.0:
    All packages built successfully! (All logs)

  • arrow/7.0.0:
    All packages built successfully! (All logs)

  • arrow/8.0.0:
    All packages built successfully! (All logs)

  • arrow/10.0.0:
    All packages built successfully! (All logs)

  • arrow/8.0.1:
    All packages built successfully! (All logs)


Conan v2 pipeline ✔️

Note: Conan v2 builds may be required once they are on the v2 ready list

All green in build 1 (283ac516c2ef0aee54cc7c40c0058ba5a0289b33):

  • arrow/12.0.0:
    All packages built successfully! (All logs)

  • arrow/10.0.0:
    All packages built successfully! (All logs)

  • arrow/8.0.1:
    All packages built successfully! (All logs)

  • arrow/8.0.0:
    All packages built successfully! (All logs)

  • arrow/1.0.0:
    All packages built successfully! (All logs)

  • arrow/7.0.0:
    All packages built successfully! (All logs)

  • arrow/2.0.0:
    All packages built successfully! (All logs)

  • arrow/11.0.0:
    All packages built successfully! (All logs)

  • arrow/10.0.1:
    All packages built successfully! (All logs)

@jcar87
Copy link
Contributor

jcar87 commented Aug 21, 2023

self.options and self.settings can no longer be accessed from package_id() in Conan v2.

I'm very open to suggestions if anybody has a better solution.

I'm not sure if the same would apply in the case of the arrow recipe - technically, having a "bogus" option value of "auto" shouldn't be needed if the config_options method is used according to the documentation.
We have a similar fix in https://github.com/conan-io/conan-center-index/pull/16177/files - although I appreciate it may not be the exact same use case, the reason it applied in that recipe is because the "auto" value was wholly dependant on settings, rather than other options.

I'm not sure auto has much value here, but this PR looks a like a good compromise, albeit an unfortunate one to have to make!

@danimtb
Copy link
Member

danimtb commented Aug 24, 2023

Hi! I didn't know there was a PR already addressing this issue. I opened one myself #19296 trying to refactor the recipe around this options issue. Please let me know what you think, thanks!

@valgur
Copy link
Contributor Author

valgur commented Aug 24, 2023

Hi! I didn't know there was a PR already addressing this issue. I opened one myself #19296 trying to refactor the recipe around this options issue. Please let me know what you think, thanks!

Oh, #19380 solves the issue perfectly. Thanks! Closing this one in favor of it.

@valgur valgur closed this Aug 24, 2023
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.

4 participants