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

Rebuild for abseil_cpp202206230 #60

Conversation

regro-cf-autotick-bot
Copy link
Contributor

This PR has been triggered in an effort to update abseil_cpp202206230.

Notes and instructions for merging this PR:

  1. Please merge the PR only after the tests have passed.
  2. Feel free to push to the bot's branch to update this PR if needed.

Please note that if you close this PR we presume that the feedstock has been rebuilt, so if you are going to perform the rebuild yourself don't close this PR until the your rebuild has been merged.

If this PR was opened in error or needs to be updated please add the bot-rerun label to this PR. The bot will close this PR and schedule another one. If you do not have permissions to add this label, you can use the phrase @conda-forge-admin, please rerun bot in a PR comment to have the conda-forge-admin add it for you.

This PR was created by the regro-cf-autotick-bot. The regro-cf-autotick-bot is a service to automatically track the dependency graph, migrate packages, and propose package version updates for conda-forge. Feel free to drop us a line if there are any issues! This PR was generated by https://github.com/regro/autotick-bot/actions/runs/2736155418, please use this URL for debugging.

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@hmaarrfk
Copy link
Contributor

I'm going to try to roll back this particular migration: conda-forge/conda-forge-pinning-feedstock#3159

@hmaarrfk hmaarrfk force-pushed the rebuild-abseil_cpp202206230-0-1_h323398 branch from 7f07e85 to edfaf0f Compare September 10, 2022 01:32
@hmaarrfk hmaarrfk added the automerge Merge the PR when CI passes label Sep 10, 2022
@github-actions
Copy link
Contributor

Hi! This is the friendly conda-forge automerge bot!

I considered the following status checks when analyzing this PR:

  • linter: passed
  • azure: failed

Thus the PR was not passing and not merged.

@h-vetinari
Copy link
Member

h-vetinari commented Sep 10, 2022

Can you replace abseil-cpp -> libabseil here & rerender? This is still using the C++11 windows builds

@hmaarrfk
Copy link
Contributor

@conda-forge-admin please rerender

@hmaarrfk
Copy link
Contributor

This is still using the C++11 windows builds

Do i need to change any compile time flags?

@h-vetinari
Copy link
Member

Do i need to change any compile time flags?

To be on the safe side, I'd add -DCMAKE_CXX_STANDARD=17 to the build scripts

@h-vetinari
Copy link
Member

Do i need to change any compile time flags?

To be on the safe side, I'd add -DCMAKE_CXX_STANDARD=17 to the build scripts

Though it depends on the project if they hardcode the c++ version somewhere, then we'd need to probably patch that out.

@h-vetinari
Copy link
Member

Something isn't working 100% with a48f26b yet. Somewhere, conda smithy still seems to find an abseil-cpp pin, otherwise they'd be removed from the CI support yamls

@h-vetinari
Copy link
Member

Something isn't working 100% with a48f26b yet. Somewhere, conda smithy still seems to find an abseil-cpp pin, otherwise they'd be removed from the CI support yamls

Found it, there's a selector on patch 19.

@h-vetinari
Copy link
Member

I checked the patch, it's fine to keep, just need to drop the selector and rerender

@hmaarrfk
Copy link
Contributor

@conda-forge-admin please rerender

@hmaarrfk
Copy link
Contributor

just need to drop the selector and rerender

I've been pretty heavy handed already with a few migrations. I don't want to change the "style" too much more than I have to.

Thanks for finding it!

@hmaarrfk
Copy link
Contributor

Do i need to change any compile time flags?

To be on the safe side, I'd add -DCMAKE_CXX_STANDARD=17 to the build scripts

Though it depends on the project if they hardcode the c++ version somewhere, then we'd need to probably patch that out.

Thats the thing, they use scons which i am not familiar with

https://github.com/conda-forge/mongodb-feedstock/blob/main/recipe/bld.bat#L27

@h-vetinari
Copy link
Member

I've been pretty heavy handed already with a few migrations. I don't want to change the "style" too much more than I have to.

No worries. Though the selector is still pointless now, because we won't be building for such an old abseil anymore, ever. 😅

@hmaarrfk
Copy link
Contributor

I searched through the old build log. You can find /std:c++17 but can't find the other c++ standards, so I think we are OK.

@hmaarrfk hmaarrfk added the automerge Merge the PR when CI passes label Sep 10, 2022
@h-vetinari
Copy link
Member

I searched through the old build log. You can find /std:c++17 but can't find the other c++ standards, so I think we are OK.

Also on windows? Then I'd say it's fine, but we should still try to eventually make that explicit

@hmaarrfk
Copy link
Contributor

Yes on windows:

2022-08-27T09:06:36.5729849Z [364+2/4024] Compiling build\opt\mongo\base\data_range_cursor.obj
2022-08-27T09:06:36.5730522Z Microsoft (R) C/C++ Optimizing Compiler Version 19.29.30146 for x64
2022-08-27T09:06:36.5731176Z Copyright (C) Microsoft Corporation.  All rights reserved.
2022-08-27T09:06:36.5731512Z 
2022-08-27T09:06:36.5734259Z cl /Fobuild\opt\mongo\base\data_range_cursor.obj /c src\mongo\base\data_range_cursor.cpp /I%PREFIX%\Library\include /I%PREFIX%\Library\include /FImongo/platform/basic.h /EHsc /W3 /wd4068 /wd4244 /wd4267 /wd4290 /wd4351 /wd4355 /wd4373 /wd4800 /wd4251 /wd4291 /we4013 /we4099 /we4930 /errorReport:none /MD /O2 /Oy- /bigobj /utf-8 /permissive- /Zc:__cplusplus /Zc:sizedDealloc /volatile:iso /diagnostics:caret /std:c++17 /Gw /Gy /Zc:inline /showIncludes /DSAFEINT_USE_INTRINSICS=0 /DBOOST_ALL_DYN_LINK /DNDEBUG /D_UNICODE /DUNICODE /D_SILENCE_CXX17_ALLOCATOR_VOID_DEPRECATION_WARNING /D_SILENCE_CXX17_OLD_ALLOCATOR_MEMBERS_DEPRECATION_WARNING /D_SILENCE_CXX17_CODECVT_HEADER_DEPRECATION_WARNING /D_SILENCE_ALL_CXX20_DEPRECATION_WARNINGS /D_CONSOLE /D_CRT_SECURE_NO_WARNINGS /D_ENABLE_EXTENDED_ALIGNED_STORAGE /D_SCL_SECURE_NO_WARNINGS /D_WIN32_WINNT=0x0A00 /DBOOST_USE_WINAPI_VERSION=0x0A00 /DNTDDI_VERSION=0x0A000000 /DBOOST_THREAD_VERSION=5 /DBOOST_THREAD_USES_DATETIME /DBOOST_SYSTEM_NO_DEPRECATED /DBOOST_MATH_NO_LONG_DOUBLE_MATH_FUNCTIONS /DBOOST_ENABLE_ASSERT_DEBUG_HANDLER /DBOOST_LOG_NO_SHORTHAND_NAMES /DBOOST_LOG_USE_NATIVE_SYSLOG /DBOOST_LOG_WITHOUT_THREAD_ATTR /DABSL_FORCE_ALIGNED_ACCESS /Isrc\third_party\variant-1.4.0\include /Isrc\third_party\SafeInt /Isrc\third_party\fmt\dist\include /Ibuild\opt /Isrc

@hmaarrfk hmaarrfk removed the automerge Merge the PR when CI passes label Sep 10, 2022
@hmaarrfk
Copy link
Contributor

I'm removing automerge so we manually check once these builds succeed

@hmaarrfk
Copy link
Contributor

Seems like windows is assuming it is using a static build and looking for lib files that are only in the static builds:

2022-09-10T17:08:57.5339758Z build\opt\third_party\IntelRDFPMathLib20U1\intel_decimal128.lib 
2022-09-10T17:08:58.0093477Z build\opt\third_party\kms-message\kms-message.lib 
2022-09-10T17:08:58.0094700Z build\opt\third_party\libstemmer_c\stemmer.lib 
2022-09-10T17:08:58.0095347Z build\opt\third_party\timelib-2021.06\timelib.lib 
2022-09-10T17:08:58.0095794Z LINK : fatal error LNK1181: cannot open input file 'absl_raw_hash_set.lib'
2022-09-10T17:08:58.0096311Z [4020+1/4024] Compiling build\opt\third_party\wiredtiger\src\txn\txn_timestamp.obj
2022-09-10T17:08:58.0096819Z Microsoft (R) C/C++ Optimizing Compiler Version 19.29.30146 for x64
2022-09-10T17:08:58.0097925Z Copyright (C) Microsoft Corporation.  All rights reserved.
2022-09-10T17:08:58.0098450Z 

@hmaarrfk
Copy link
Contributor

Well here goes an other 5 hour build cycle

@hmaarrfk hmaarrfk added the automerge Merge the PR when CI passes label Sep 10, 2022
+ 'absl_strings_internal', 'absl_base', 'absl_spinlock_wait',
+ 'absl_bad_variant_access', 'absl_raw_logging_internal',
+ 'absl_log_severity', 'absl_int128'
+ 'absl_dll'
Copy link
Member

Choose a reason for hiding this comment

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

Confusingly (or infuriatingly), this is actually called abseil_dll

Copy link
Contributor

Choose a reason for hiding this comment

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

and it only gets triggered at the way end of the build ^_^

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this patch should be reverted for unix, and updated only for windows.

Copy link
Member

@h-vetinari h-vetinari Sep 10, 2022

Choose a reason for hiding this comment

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

That was my main point, yes 🙃

Copy link
Contributor

Choose a reason for hiding this comment

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

sorry, i didn't understand the fact that you called out my earlier typo

recipe/meta.yaml Outdated
@@ -26,8 +26,7 @@ source:
- patches/0016-Disable-aligned-allocation-on-macOS.patch
- patches/0017-Drop-strict-python-pkgs-requirements-check.patch
- patches/0018-Fix-flags-for-un-vendoring-abseil-cpp.patch
- patches/0019-wyhash-is-now-called-low_level_hash.patch # [libabseil != '20210324.2']
- patches/0020-Don-t-inject-ICU-Macros-if-using-pre-compiled-librar.patch
- patches/0019-Don-t-inject-ICU-Macros-if-using-pre-compiled-librar.patch
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- patches/0019-Don-t-inject-ICU-Macros-if-using-pre-compiled-librar.patch
- patches/0019-Don-t-inject-ICU-Macros-if-using-pre-compiled-librar.patch # [win]

Copy link
Contributor

Choose a reason for hiding this comment

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

but this patch i didn't change.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, then I meant that it should go to 18; in any case, my point was that the patch that switches to abseil_dll should only apply to windows.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, yeah, lets get a working build for windows :D.

@hmaarrfk hmaarrfk force-pushed the rebuild-abseil_cpp202206230-0-1_h323398 branch from c65bb4c to 79cc775 Compare September 11, 2022 01:16
@github-actions github-actions bot removed the automerge Merge the PR when CI passes label Sep 11, 2022
@github-actions
Copy link
Contributor

Hi! This is the friendly conda-forge automerge bot!

Commits were made to this PR after the automerge label was added. For security reasons, I have disabled automerge by removing the automerge label. Please add the automerge label again (or ask a maintainer to do so) if you'd like to enable automerge again!

@hmaarrfk hmaarrfk added the automerge Merge the PR when CI passes label Sep 11, 2022
@h-vetinari
Copy link
Member

These static data member linkage errors are always baffling to me, but IIRC correctly, it should be solved by adding something like conda-forge/google-cloud-cpp-feedstock@c1d4123 (which came from here but ended up being rebased out from the PR that superseded it, since it wasn't necessary for building a static lib).

@github-actions github-actions bot removed the automerge Merge the PR when CI passes label Sep 11, 2022
@github-actions
Copy link
Contributor

Hi! This is the friendly conda-forge automerge bot!

Commits were made to this PR after the automerge label was added. For security reasons, I have disabled automerge by removing the automerge label. Please add the automerge label again (or ask a maintainer to do so) if you'd like to enable automerge again!

@hmaarrfk
Copy link
Contributor

@conda-forge-admin please rerender

@hmaarrfk
Copy link
Contributor

Just avoiding needless builds since it seems that boost will get merged first.

@hmaarrfk hmaarrfk mentioned this pull request Sep 11, 2022
5 tasks
@h-vetinari
Copy link
Member

Seems this passed now? Was it just a restart that was needed to get a faster CI agent?

@hmaarrfk
Copy link
Contributor

Builds have been oscillating between 275 min and 350 + minutes. Sometimes it would only get to 3200/4000 and other times it would pass.

@h-vetinari
Copy link
Member

Builds have been oscillating between 275 min and 350 + minutes.

That's annoying but not a deal-breaker. Just needs occasional CI restarts. If you want you can add me to maintainers and I'll do it.

@hmaarrfk hmaarrfk force-pushed the rebuild-abseil_cpp202206230-0-1_h323398 branch from 99e19dd to 38820f4 Compare September 12, 2022 16:39
@h-vetinari
Copy link
Member

I would have thought that based on your work in conda-forge/abseil-cpp-feedstock#49, 8ccb3b6 would not be necessary anymore...?

@h-vetinari
Copy link
Member

I would have thought that based on your work in conda-forge/abseil-cpp-feedstock#49, 8ccb3b6 would not be necessary anymore...?

But fine to leave for a follow-up...

@hmaarrfk hmaarrfk force-pushed the rebuild-abseil_cpp202206230-0-1_h323398 branch from 38820f4 to e0f9848 Compare September 12, 2022 17:03
@hmaarrfk
Copy link
Contributor

But fine to leave for a follow-up...

Sorry i forgot in my quick cleanup.

@hmaarrfk
Copy link
Contributor

I take a very big shotgun approach and i forgot that different things pan out.

@h-vetinari
Copy link
Member

All green 🚀

@hmaarrfk hmaarrfk merged commit 93f4d21 into conda-forge:main Sep 13, 2022
@regro-cf-autotick-bot regro-cf-autotick-bot deleted the rebuild-abseil_cpp202206230-0-1_h323398 branch September 13, 2022 10:27
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