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

oatpp: add version 1.3.0-latest #23696

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

toge
Copy link
Contributor

@toge toge commented Apr 22, 2024

Specify library name and version: oatpp/1.3.0-latest

oatpp/oatpp@1.3.0...1.3.0-latest


@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@AbrilRBS
Copy link
Member

🤔 wondering why upstream chose this versioning scheme and did not bump the version! Now the -latest bit will be treated as a prerelease, so anyone requiring on this by a version range will find that the new version you're adding comes before the already existing 1.3.0, even if it's newer!

@AbrilRBS AbrilRBS self-assigned this Apr 22, 2024
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! The tag is creating some issues, let's think about how we can best tackle this. Probably changing the tag to 1.3.0.latest should be enough

@@ -49,6 +50,10 @@ def validate(self):
if self.settings.compiler == "gcc" and Version(self.settings.compiler.version) < "5":
raise ConanInvalidConfiguration("oatpp requires GCC >=5")

def build_requirements(self):
if Version(self.version) >= "1.3.0":
Copy link
Member

Choose a reason for hiding this comment

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

This does not sadly work as you expect. Version("1.3.0-latest") < Version("1.3.0") because of prerelease syntax :(

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@RubenRBS
Thanks!
I add _version property to fix it.

@@ -59,6 +64,8 @@ def generate(self):
if is_msvc(self) and Version(self.version) >= "1.3.0":
tc.variables["OATPP_MSVC_LINK_STATIC_RUNTIME"] = is_msvc_static_runtime(self)
tc.generate()
venv = VirtualBuildEnv(self)
Copy link
Member

Choose a reason for hiding this comment

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

We also probably want to set OATPP_LINK_TEST_LIBRARY to false in the CMakeToolchain here

Copy link
Contributor Author

@toge toge Apr 23, 2024

Choose a reason for hiding this comment

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

@RubenRBS
Thanks a lot!
I add with_test_library option and use it's value for OATPP_LINK_TEST_LIBRARY.

@conan-center-bot

This comment has been minimized.

Copy link
Contributor

Hooks produced the following warnings for commit 05ce03b
oatpp/1.3.0-latest@#fb75cf3cef49ca7de53b98077672f399
post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/oatpp-1.3.0/liboatpp.so' links to system library 'm' but it is not in cpp_info.system_libs.
oatpp/1.3.0@#1977cebedac494f91f44f0f112c3bde4
post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/oatpp-1.3.0/liboatpp.so' links to system library 'm' but it is not in cpp_info.system_libs.
post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/oatpp-1.3.0/liboatpp-test.so' links to system library 'm' but it is not in cpp_info.system_libs.
oatpp/1.2.5@#0ef2fc2eaac97c4d7d66879c2e37e4ea
post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/oatpp-1.2.5/liboatpp.so' links to system library 'm' but it is not in cpp_info.system_libs.
post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/oatpp-1.2.5/liboatpp-test.so' links to system library 'm' but it is not in cpp_info.system_libs.
oatpp/1.2.0@#33ed5d1154cc2087ede6a635a09d8f8f
post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/oatpp-1.2.0/liboatpp.so' links to system library 'm' but it is not in cpp_info.system_libs.
post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/oatpp-1.2.0/liboatpp-test.so' links to system library 'm' but it is not in cpp_info.system_libs.
oatpp/1.1.0@#af84ccf6805f8795d54c59ee06b877eb
post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/oatpp-1.1.0/liboatpp.so' links to system library 'm' but it is not in cpp_info.system_libs.
post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/oatpp-1.1.0/liboatpp-test.so' links to system library 'm' but it is not in cpp_info.system_libs.
oatpp/1.0.0@#a9778d66762d5c419d1bc8a8c49cdea8
post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/oatpp-1.0.0/liboatpp.so' links to system library 'm' but it is not in cpp_info.system_libs.
post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library './lib/oatpp-1.0.0/liboatpp-test.so' links to system library 'm' but it is not in cpp_info.system_libs.

@conan-center-bot
Copy link
Collaborator

Conan v1 pipeline ✔️

All green in build 4 (f899e8c5110c5268f37fb17cf0defdd7b10c5afb):

  • oatpp/1.3.0-latest:
    All packages built successfully! (All logs)

  • oatpp/1.3.0:
    All packages built successfully! (All logs)

  • oatpp/1.2.5:
    All packages built successfully! (All logs)

  • oatpp/1.1.0:
    All packages built successfully! (All logs)

  • oatpp/1.2.0:
    All packages built successfully! (All logs)

  • oatpp/1.0.0:
    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 4 (f899e8c5110c5268f37fb17cf0defdd7b10c5afb):

  • oatpp/1.3.0-latest:
    All packages built successfully! (All logs)

  • oatpp/1.3.0:
    All packages built successfully! (All logs)

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

  • oatpp/1.2.0:
    All packages built successfully! (All logs)

  • oatpp/1.1.0:
    All packages built successfully! (All logs)

  • oatpp/1.2.5:
    All packages built successfully! (All logs)

Copy link
Contributor

@perseoGI perseoGI left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@perseoGI perseoGI left a comment

Choose a reason for hiding this comment

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

Modify version to make Version("1.3.0_latest") > Version("1.3.0")

@@ -1,4 +1,7 @@
sources:
"1.3.0-latest":
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"1.3.0-latest":
"1.3.0_latest":

Modify version to make Version("1.3.0_latest") > Version("1.3.0")

Comment on lines +38 to +39
if version.endswith("-latest"):
version = version[:-len("-latest")]
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if version.endswith("-latest"):
version = version[:-len("-latest")]
if version.endswith("_latest"):
version = version[:-len("_latest")]

@@ -1,4 +1,6 @@
versions:
"1.3.0-latest":
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"1.3.0-latest":
"1.3.0_latest":

@ErniGH ErniGH mentioned this pull request Sep 18, 2024
3 tasks
@DannyLihard
Copy link

Hello, can we make the last necessary changes to merge this PR ? I'd like to use this version via Conan, and I intend to create PRs for some of Oat++'s extensions too.

I agree with you about the bad versioning scheme, not helping there. Another solution would be to create a recipe v1.3.1 that points to Oat++'s 1.3.0-latest tag, now that we know the next official release will be v1.4. While a bit confusing for users (still, the information can easily be found in conandata.yml), this would be easier (simply patching the #defined version in source code) than modifying the recipe for specific version string suffixes.
Moreover, these modifications would be similar for all extensions' PRs (oatpp-openssl, oatpp-postgresql, etc.). So the easy 1.3.1 workaround would be the same in those, as opposed as the recipe modification in all of them.

What do you think ?

@perseoGI
Copy link
Contributor

The thing is that, given the conan Version behavior:

>>> from conan.tools.scm import Version
>>> Version("1.3.0-latest") > Version("1.3.1")
False
>>> Version("1.3.0-latest") > Version("1.3.0")
False
>>> Version("1.3.0-latest") < Version("1.3.0")
True
>>> Version("1.3.0_latest") > Version("1.3.1")
False
>>> Version("1.3.0_latest") > Version("1.3.0")
True

If we use 1.3.0-latest tag, we will have to deal with this problems. Thus, it is better to rename the tag to the underscore one @toge

@DannyLihard
Copy link

The thing is that, given the conan Version behavior:

>>> from conan.tools.scm import Version
>>> Version("1.3.0-latest") > Version("1.3.1")
False
>>> Version("1.3.0-latest") > Version("1.3.0")
False
>>> Version("1.3.0-latest") < Version("1.3.0")
True
>>> Version("1.3.0_latest") > Version("1.3.1")
False
>>> Version("1.3.0_latest") > Version("1.3.0")
True

If we use 1.3.0-latest tag, we will have to deal with this problems. Thus, it is better to rename the tag to the underscore one @toge

That's why I'm proposing to use neither 1.3.0-latest nor 1.3.0_latest from the Conan point of view, but 1.3.1 instead.
I think that if we want to merge this PR, we need to either :

  • Have the changes you proposed done (use 1.3.0_latest in Conan).
  • Go for the 1.3.1 solution.

Both these solutions would use the 1.3.0-latest Git tag of the oatpp repository behind the scenes anyway.

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