Restructure directories add 2.3.4 #8561
Conversation
This comment has been minimized.
This comment has been minimized.
recipes/fast-dds/2.3.X/conanfile.py
Outdated
| @@ -176,10 +178,10 @@ def package(self): | |||
|
|
|||
| def package_info(self): | |||
| self.cpp_info.names["cmake_find_package"] = "fastdds" | |||
| self.cpp_info.names["cmake_find_package_multi"] = "fastdds" | |||
| self.cpp_info.names["cmake_find_multi_package"] = "fastdds" | |||
recipes/fast-dds/2.3.X/conanfile.py
Outdated
| self.copy("CMakeLists.txt") | ||
| for patch in self.conan_data.get("patches", {}).get(self.version, []): | ||
| self.copy(patch["patch_file"]) | ||
| self.requires("openssl/1.1.1k") |
recipes/fast-dds/2.3.X/conanfile.py
Outdated
| def export_sources(self): | ||
| self.copy("CMakeLists.txt") | ||
| for patch in self.conan_data.get("patches", {}).get(self.version, []): | ||
| self.copy(patch["patch_file"]) |
There was a problem hiding this comment.
clicked wrong button .... sorry for that will soon add again
recipes/fast-dds/config.yml
Outdated
| folder: 2.3.X | ||
| "2.3.3": | ||
| folder: all | ||
| folder: 2.3.X | ||
| "2.3.4": | ||
| folder: 2.3.X |
There was a problem hiding this comment.
in next PR want to add 2.4.X and 2.5.X and they differ in some regions
There was a problem hiding this comment.
Are those regions too big? Sometimes it is better to have some big if/else that to have several conanfile.py files that will start to diverge. Do you already have a draft for those changes?
There was a problem hiding this comment.
@jgsogo i will check it again and rechange it back to all if not too different.
There was a problem hiding this comment.
@jgsogo i found my local 2.4.X again - the difference indeed is truly just related to the patches - i will look again other conanfiles how to handle that. 2.4.X dont need patches 2.3.X needs. guess i should check the version before apply the patches inside conanfile - if that is the proper way
There was a problem hiding this comment.
ok renamed back to all
recipes/fast-dds/2.3.X/conanfile.py
Outdated
| def validate(self): | ||
| # FIXME: Foonathan dependency currently not proper working with cross compile | ||
| # see https://github.com/conan-io/conan-center-index/pull/7632#discussion_r730445887 | ||
| if hasattr(self, "settings_build") and tools.cross_building(self): | ||
| raise ConanInvalidConfiguration("Cross building is not yet supported. Contributions are welcome.") | ||
|
|
There was a problem hiding this comment.
to remove, there is already a validate() method. It seems you have reverted all the fixes of another PR, correct?
There was a problem hiding this comment.
yes - adapted and try again
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
recipes/fast-dds/2.3.X/conanfile.py
Outdated
| elif self.settings.os == "Macos": | ||
| # FIXME: Testing currently fails with MacOS - see https://c3i.jfrog.io/c3i/misc/summary.html?json=https://c3i.jfrog.io/c3i/misc/logs/pr/8561/1-configs/macos-clang/fast-dds/2.3.2//summary.json | ||
| # dyld: Library not loaded: libfastrtps.2.3.dylib --> Reason: image not found | ||
| raise ConanInvalidConfiguration("Macos currently not supported") |
There was a problem hiding this comment.
I'm not aware of any change on the Macos machines, this should work as it was working before 🤔
There was a problem hiding this comment.
Also extremely strange to me - but have no Mac - so cannot check reason ....
There was a problem hiding this comment.
I've just tried locally and it worked for me (apple-clang 13). I'll investigate it in a different PR here in CCI
There was a problem hiding this comment.
Here is the link: #8566. I'm also upgrading dependencies, to apply some meaningful change.
There was a problem hiding this comment.
@jgsogo thanks for supporting here :)
There was a problem hiding this comment.
No but your test package is likely broken, you don't inject DYLD_LIBRARY_PATH by not relying on self.run with environment=True, so yes shared on Macos breaks obviously.
Why is it so complex now? test_package is not supposed to be a functional test of the library.
There was a problem hiding this comment.
1st thanks for pointing it out - yes -.- i forgot i start them with popen now -.- will adapt tomorrow.
2nd The reason for this huge test is a bug hard to detect but happened when one dependency (foonathan memory) was set with wrong default options. then sending messages from one thread to one other inside same process works fine but the communication between 2 process crash on runtime. Basically i want to check if 1 message can be sent by one process and received by 1 process. i dont want accidentally again a package with this misbehavior.
This comment has been minimized.
This comment has been minimized.
|
I detected other pull requests that are modifying fast-dds/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. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Thank you for your contribution! Please, keep test package as simple as possible, don't need to run a full functional example, only a simple test to validate the library linkage, header location and cmake targets. Your new test is too complex and requires networking. I suggest you reverting your changes over test package and update only the new version and its patch. Happy new year! |
|
@uilianries as you wish. |
|
@uilianries ok test_package now back to original. in the next PR i will add 2.4 and after 2.5 series. and btw - happy new year to you too :) |
|
Might want to update the PR title so we have a nice commit message when the bot merges |
| @@ -146,7 +146,6 @@ def validate(self): | |||
| # linking dynamic '*.dll' and static MT runtime | |||
| raise ConanInvalidConfiguration("Mixing a dll {} library with a static runtime is a bad idea".format(self.name)) | |||
|
|
|||
|
|
|||
There was a problem hiding this comment.
This modification in conanfile is unnecessary. Remove it and CI could detect a bump.
There was a problem hiding this comment.
I dont think the bot can since there is a patch... Unless it got wicked smart (there's some smart people on the Conan team)
There was a problem hiding this comment.
it can, thanks to:
def export_sources(self):
self.copy("CMakeLists.txt")
for patch in self.conan_data.get("patches", {}).get(self.version, []):
self.copy(patch["patch_file"])therefore, RREV of other versions is not modified even with a patch in the new version, so the bot can detect a bump (it relies on RREV) if there are no modifications in conanfile or other files in export_sources of these versions.
There was a problem hiding this comment.
🤯 I'll need to improve my bot then! Thanks for the news
Yes, just |
dotChris90
changed the title
|
but too late -.- i will take care next PR sorry |
* rename all folder to 2.3.X * add 2.3.4 and adding better tests * exclude Macos * typo * increase to 30s * rename back to all * Mac requires environment variables * removed again to check test now works on mac * because conan internal functions shall not be used * just try execution on mac os * just eval assert on linux .... because windows sometimes error * simplify test according to PR commits
Fast-DDS/2.3.X
first step for old PR #7853
conan-center hook activated.