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.

Sign up for GitHub

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

Jump to bottom

fixed_math: add recipe #24685

Merged
merged 19 commits into from
Sep 4, 2024
Merged

fixed_math: add recipe #24685

merged 19 commits into from
Sep 4, 2024

Conversation

toge
Copy link
Contributor

@toge toge commented Jul 22, 2024

Summary

Changes to recipe: fixed_math/1.0.1

Motivation

fixed_math is a library for fix-point numeric math library.

Details

https://github.com/arturbac/fixed_math

@conan-center-bot

This comment has been minimized.

Sign in to view
@conan-center-bot

This comment has been minimized.

Sign in to view
@conan-center-bot

This comment has been minimized.

Sign in to view
@conan-center-bot

This comment has been minimized.

Sign in to view
@conan-center-bot

This comment has been minimized.

Sign in to view
@conan-center-bot

This comment has been minimized.

Sign in to view
@conan-center-bot

This comment has been minimized.

Sign in to view
@conan-center-bot

This comment has been minimized.

Sign in to view
@conan-center-bot

This comment has been minimized.

Sign in to view
@conan-center-bot

This comment has been minimized.

Sign in to view
@conan-center-bot

This comment has been minimized.

Sign in to view
@github-actions GitHub Actions
Copy link
Contributor

Hooks produced the following warnings for commit 60c18fb
fixed_math/1.0.1@#505c334b5cfddabd57ba11cf9e4e9c3a 
post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/libfixed_math.so' links to system library 'm' but it is not in cpp_info.system_libs.

@AbrilRBS AbrilRBS self-assigned this Aug 5, 2024
AbrilRBS
AbrilRBS reviewed Aug 5, 2024
View reviewed changes
Copy link
Member

@AbrilRBS AbrilRBS left a comment

Choose a reason for hiding this comment

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

Thanks! Some comments :)

recipes/fixed_math/all/conanfile.py Outdated
def build(self):
replace_in_file(self, os.path.join(self.source_folder, "fixed_lib", "include", "fixedmath", "fixed_math.hpp"),
"template<typename arithmethic_type>\n constexpr fixed_t arithmetic_to_fixed( arithmethic_type value ) noexcept;",
"template<typename arithmethic_type, typename>\n constexpr fixed_t arithmetic_to_fixed( arithmethic_type value ) noexcept;"
Copy link
Member

Choose a reason for hiding this comment

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

Any insight into this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@AbrilRBS
Thank you for your comment.
I add comment for this patch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This patch has been merged into upstream.

recipes/fixed_math/all/conanfile.py Outdated
tc = CMakeToolchain(self)
if is_msvc(self):
tc.variables["CMAKE_CXX_FLAGS"] = "/Zc:__cplusplus"
tc.variables["CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS"] = True
Copy link
Member

Choose a reason for hiding this comment

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

Is this supported by upstream? Our policy on CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS is to not use it and disable shared support unless upstream specifically mentions it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@AbrilRBS
I see.
I remove these lines and add validation logic for windows shared build.

@conan-center-bot

This comment has been minimized.

Sign in to view
@conan-center-bot

This comment has been minimized.

Sign in to view
valgur
valgur reviewed Aug 6, 2024
View reviewed changes
recipes/fixed_math/all/conanfile.py Outdated
Comment on lines 78 to 79
if self.settings.os == "Windows" and self.options.get_safe("shared"):
raise ConanInvalidConfiguration(f"{self.ref} does not support shared builds on Windows")
Copy link
Contributor

Choose a reason for hiding this comment

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

As suggested by @uilianries in another PR, this can be replaced with

    self.package_type = "static-library"
    del self.options.shared

under config_options(), which allows the package to be used with */*:shared=True on Windows.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@valgur
Thank a lot!
Now I see. Fixed.

@conan-center-bot

This comment has been minimized.

Sign in to view
@toge toge changed the title fixed_math: add recipe fixed_math: add new recipe Aug 10, 2024
@conan-center-bot

This comment has been minimized.

Sign in to view
@conan-center-bot

This comment has been minimized.

Sign in to view
@conan-center-bot

This comment has been minimized.

Sign in to view
@toge toge changed the title fixed_math: add new recipe fixed_math: add recipe Sep 3, 2024
@toge
Copy link
Contributor Author

toge commented Sep 4, 2024

@AbrilRBS
The CI pipeline doesn't seem to be working properly.
Is there a solution that I can do?

@AbrilRBS
Copy link
Member

AbrilRBS commented Sep 4, 2024

Sorry for that @toge, this is now running properly :)

@conan-center-bot
Copy link
Collaborator

Conan v1 pipeline ✔️

All green in build 23 (38e0cfa052b267e933d6cdde95d7a91e58432b05):

  • fixed_math/1.0.2:
    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 23 (38e0cfa052b267e933d6cdde95d7a91e58432b05):

  • fixed_math/1.0.2:
    All packages built successfully! (All logs)

AbrilRBS
AbrilRBS approved these changes Sep 4, 2024
View reviewed changes
Copy link
Member

@AbrilRBS AbrilRBS left a comment

Choose a reason for hiding this comment

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

Thanks @toge :)

@AbrilRBS AbrilRBS requested a review from ErniGH September 4, 2024 08:21
ErniGH
ErniGH approved these changes Sep 4, 2024
View reviewed changes
Copy link
Contributor

@ErniGH ErniGH left a comment

Choose a reason for hiding this comment

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

Thanks ❤️

@conan-center-bot conan-center-bot merged commit 6e66db4 into conan-io:master Sep 4, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@valgur valgur valgur left review comments

@AbrilRBS AbrilRBS AbrilRBS approved these changes

@ErniGH ErniGH ErniGH approved these changes

Assignees

@AbrilRBS AbrilRBS

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@toge @conan-center-bot @AbrilRBS @valgur @ErniGH