-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Add aws-sdk-cpp/1.7.299 recipe #1146
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
related to #621 |
@madebr any updates on this? |
I just pushed a fix to #1140 |
This comment has been minimized.
This comment has been minimized.
CI needs to be re-triggered? |
ping |
let's try again |
This comment has been minimized.
This comment has been minimized.
@danimtb something is wrong here, jenkins is hanging, could you please check? |
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
@danimtb how can we push this forward? |
An unexpected error happened and has been reported. Help is on its way! 🏇 |
one thing that I think we need to address in the conan center is the process of adding a package: aws-sdk-cpp adds a release every day!! now as CCI is not automated (yet?) and this recipe is taking months to merge: I think that now no one is going to use version are there concrete plans to allow people to do the same as npm or cargo does? e.g. upload a package to the CCI by themselfs e.g. when they are upstream? |
@gocarlos That's a good point for a further discussion. Usually we include new version when someone asks, for example, Facebook Folly has no releases, so we tag by the date. |
@madebr I see the build of this package takes a lot of memory usage in our containers and it gets killed. I suppose aws-sdk-cpp is a huge project. Currently we have a 4 GB limit of memory for runtime processes, could you verify if you are seeing this values as well from your side? |
@madebr is mining bitcoins in CCI server 😹 |
@uilianries The question should be: has anyone done this already? 😺 |
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.
Please add |
This comment has been minimized.
This comment has been minimized.
I think it was killed due to out-of-memory. |
Some configurations of 'aws-sdk-cpp/1.8.34' failed in build 22 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add build_only
option to shorten build times
"use_virtual_operations": [True, False], | ||
"with_curl": [True, False], | ||
"enable_curl_logging": [True, False], | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
} | |
"build_only": "ANY", | |
} |
"use_virtual_operations": True, | ||
"with_curl": True, | ||
"enable_curl_logging": False, | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
} | |
"build_only": None, | |
} |
self._cmake.definitions["ENABLE_CURL_LOGGING"] = self.options.get_safe("enable_curl_logging") | ||
if self.settings.compiler.cppstd: | ||
self._cmake.definitions["CPP_STANDARD"] = self._cmake_cpp_standard | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if self.options.build_only: | |
self._cmake.definitions["BUILD_ONLY"] = self.options.build_only | |
cmake = self._configure_cmake() | ||
cmake.install() | ||
|
||
tools.rmdir(os.path.join(self.package_folder, "lib", "cmake")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why delete this? I, as a consumer, prefer the library's cmake config script, if available, rather than conan's generated one
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's cci policy to remove these.
But I agree to your sentiment.
Most cmake config scripts now are relocatable, whereas in the past they had absolute paths to the build systems embedded into them.
@danimtb @uilianries @SpaceIm @jgsogo
aws-sdk-cpp provides a lot of libraries (and config cmake scripts). By a lot, I mean 30+.
conan does currently not provide a way to generate multiple config scripts.
And even then, the generated conan cmake scripts will be generic and never the same as those provided by the aws-sdk-cpp cmake build script.
I feel cci should allow those cmake scripts that play by the book and are fully relocatable.
This would also save a lot of maintenance and acceptance of conan/cci.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that the other AWS packages don't delete this folder, e.g.
self.cpp_info.components["aws-c-common-lib"].builddirs = [os.path.join("lib", "cmake")] |
if self.settings.compiler.cppstd: | ||
tools.check_min_cppstd(self, 2003) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why 2003? Don't you mean 11 (looking at build()
)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering myself.
https://github.com/aws/aws-sdk-cpp/blob/2dffafc32c419bc6be45d6345fef0cd50eec3bd3/CMakeLists.txt#L73 says current is 11.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keep 11. 2003 it's not listed as default cppstd in setting.yml.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll make the change to 11.
But 2003 is a fine value too.
Last time I checked, tools.check_min_cppstd
converts self.settings.compiler.cppstd
and 11
to a 4-digit year,
That way comparing 98
and 11
uses the same code path.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Btw, using 98,11,14,17 and 20 will cause a new y2k-kind problem in less then 80 years. 🤣
def requirements(self): | ||
self.requires("aws-c-common/0.4.25") | ||
self.requires("aws-c-event-stream/0.1.5") | ||
self.requires("openssl/1.1.1g") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
self.requires("openssl/1.1.1g") | |
self.requires("openssl/1.1.1h") |
self.requires("openssl/1.1.1g") | ||
self.requires("zlib/1.2.11") | ||
if self.options.with_curl: | ||
self.requires("libcurl/7.71.1") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
self.requires("libcurl/7.71.1") | |
self.requires("libcurl/7.73.0") |
for root, _, _ in os.walk(self._source_subfolder): | ||
cmakelists = os.path.join(root, "CMakeLists.txt") | ||
repl = "AWS::aws-c-event-stream" | ||
if os.path.isfile(cmakelists) and repl in tools.load(cmakelists): | ||
tools.replace_in_file(cmakelists, repl, "CONAN_PKG::aws-c-event-stream", strict=False) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't be required anymore, imported target in aws-c-event-stream
recipe was fixed in #2584
tools.rmdir(os.path.join(self.package_folder, "lib", "pkgconfig")) | ||
|
||
def package_info(self): | ||
self.cpp_info.libs = tools.collect_libs(self) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as there are inter-dependencies between components (e.g. many depend on aws-sdk-core
) it's not good to use collect_libs
(see https://docs.conan.io/en/latest/reference/tools.html?highlight=inter%20dependent#tools-collect-libs)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it needs a similar approach as my boost components pr (#2097)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wow, you have a lot going on
Sorry I push a new PR without noticing one was already on its way. Is there something blocking this to be merge ? |
I detected other pull requests that are modifying aws-sdk-cpp/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. |
@MartinDelille |
Failure in build 23 (
Note: To save resources, CI tries to finish as soon as an error is found. For this reason you might find that not all the references have been launched or not all the configurations for a given reference. Also, take into account that we cannot guarantee the order of execution as it depends on CI workload and workers availability. |
Specify library name and version: aws-sdk-cpp/1.7.299
conan-center hook activated.
Fixes conan-io/wishlist#145