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

Compilation in a macOS GitHub runner fails with cc v1.0.84 #902

Closed
twistedfall opened this issue Nov 14, 2023 · 19 comments · Fixed by #932 or #943
Closed

Compilation in a macOS GitHub runner fails with cc v1.0.84 #902

twistedfall opened this issue Nov 14, 2023 · 19 comments · Fixed by #932 or #943

Comments

@twistedfall
Copy link

twistedfall commented Nov 14, 2023

Hello!

For my crate opencv I see that starting with cc version 1.0.84 the macOS runners are failing: https://github.com/twistedfall/opencv-rust/actions/runs/6853515550

I must also note that I haven't used cc versions 1.0.80..=1.0.83 because of the another issue that was causing hangs due to the interaction of jobserver in both cc and opencv (#844)

The specific error is:

fatal error: 'limits' file not found

it is caused by the missing stdlib which clang warns about too:

clang: warning: include path for libstdc++ headers not found; pass '-stdlib=libc++' on the command line to use the libc++ standard library instead [-Wstdlibcxx-not-found]

As far as I can see between version 1.0.79 and 1.0.84 the C++ compiler is called with different arguments:

  • 1.0.79
"c++" "-O0" "-ffunction-sections" "-fdata-sections" "-fPIC" "-gdwarf-2" "-fno-omit-frame-pointer" "-m64" "-arch" "x86_64" "-I" "/Users/runner/work/opencv-rust/opencv-rust/src_cpp" "-I" "/Users/runner/work/opencv-rust/opencv-rust/target/debug/build/opencv-fb8aea6a002a5c33/out" "-I" "." "-I" "/usr/local/opt/opencv/include/opencv4" "-Wall" "-Wextra" "-std=c++14" "-Wno-deprecated-declarations" "-Wno-deprecated-copy" "-Wno-unused-parameter" "-Wno-sign-compare" "-Wno-comment" "-Wno-unused-variable" "-Wno-ignored-qualifiers" "-Wno-return-type-c-linkage" "-Wno-overloaded-virtual" "-F/usr/local/opt/opencv/include/opencv4" "-o" "/Users/runner/work/opencv-rust/opencv-rust/target/debug/build/opencv-fb8aea6a002a5c33/out/d25e1c8da0c38997-core.o" "-c" "/Users/runner/work/opencv-rust/opencv-rust/target/debug/build/opencv-fb8aea6a002a5c33/out/core.cpp"
  • 1.0.84
"c++" "-O0" "-ffunction-sections" "-fdata-sections" "-fPIC" "-gdwarf-2" "-fno-omit-frame-pointer" "-m64" "--target=x86_64-apple-darwin" "-mmacosx-version-min=10.7" "-I" "/Users/runner/work/opencv-rust/opencv-rust/src_cpp" "-I" "/Users/runner/work/opencv-rust/opencv-rust/target/debug/build/opencv-80956898113978e8/out" "-I" "." "-I" "/usr/local/opt/opencv/include/opencv4" "-Wall" "-Wextra" "-std=c++14" "-o" "/Users/runner/work/opencv-rust/opencv-rust/target/debug/build/opencv-80956898113978e8/out/02e9e909b56518ad-core.o" "-c" "/Users/runner/work/opencv-rust/opencv-rust/target/debug/build/opencv-80956898113978e8/out/core.cpp"

The main differences I see is the -arch, --target, -mmacosx-version-min and a bunch of -W flags. The compiler identity looks different too:

  • 1.0.79 identifiers as Gnu
=== Compiler information: Tool {
    path: "c++",
    cc_wrapper_path: None,
    cc_wrapper_args: [],
    args: [
        "-O0",
        "-ffunction-sections",
        "-fdata-sections",
        "-fPIC",
        "-gdwarf-2",
        "-fno-omit-frame-pointer",
        "-m64",
        "-arch",
        "x86_64",
        "-I",
        "/Users/runner/work/opencv-rust/opencv-rust/src_cpp",
        "-I",
        "/Users/runner/work/opencv-rust/opencv-rust/target/debug/build/opencv-fb8aea6a002a5c33/out",
        "-I",
        ".",
        "-I",
        "/usr/local/opt/opencv/include/opencv4",
        "-Wall",
        "-Wextra",
        "-std=c++14",
        "-Wno-deprecated-declarations",
        "-Wno-deprecated-copy",
        "-Wno-unused-parameter",
        "-Wno-sign-compare",
        "-Wno-comment",
        "-Wno-unused-variable",
        "-Wno-ignored-qualifiers",
        "-Wno-return-type-c-linkage",
        "-Wno-overloaded-virtual",
        "-F/usr/local/opt/opencv/include/opencv4",
    ],
    env: [],
    family: Gnu,
    cuda: false,
    removed_args: [],
}
  • 1.0.84 identifies as Clang:
=== Compiler information: Tool {
    path: "c++",
    cc_wrapper_path: None,
    cc_wrapper_args: [],
    args: [
        "-O0",
        "-ffunction-sections",
        "-fdata-sections",
        "-fPIC",
        "-gdwarf-2",
        "-fno-omit-frame-pointer",
        "-m64",
        "--target=x86_64-apple-darwin",
        "-mmacosx-version-min=10.7",
        "-I",
        "/Users/runner/work/opencv-rust/opencv-rust/src_cpp",
        "-I",
        "/Users/runner/work/opencv-rust/opencv-rust/target/debug/build/opencv-80956898113978e8/out",
        "-I",
        ".",
        "-I",
        "/usr/local/opt/opencv/include/opencv4",
        "-Wall",
        "-Wextra",
        "-std=c++14",
    ],
    env: [],
    family: Clang,
    cuda: false,
    removed_args: [],
    has_internal_target_arg: false,
}

The compiler is created as follows in opencv code: https://github.com/twistedfall/opencv-rust/blob/730cc64955ab4ad5286bee891e795a1322b053c0/build.rs#L223-L276

@twistedfall twistedfall changed the title Compilation on in a macOS GitHub runner fails with cc v1.0.84 Compilation in a macOS GitHub runner fails with cc v1.0.84 Nov 14, 2023
@NobodyXu
Copy link
Collaborator

I think it is related to #900 .
@twistedfall Can you try using the main branch of cc and see if #901 fixed this issue please?

@twistedfall
Copy link
Author

The main branch seems to work (https://github.com/twistedfall/opencv-rust/actions/runs/6864735405/job/18667178550), although I do see a "macOS deployment target (10.7) too low, it will be increased" warning on each c++ invocation. Additionally it seems that it's running c++ --version a lot now.

@ahayzen-kdab
Copy link

We have the same issue with CXX-Qt, 1.0.83 works but 1.0.84 doesn't ( KDAB/cxx-qt#733 ). I'll see if main branch works or not for us too.

@ahayzen-kdab
Copy link

So the issue changed using main, now i get the following in CI instead of the clang: warning: include path for libstdc++ headers not found; pass '-stdlib=libc++' on the command line to use the libc++ standard library instead issue before. We are using macOS 11 at the moment 🤔

 warning: /Users/runner/cxx-qt/build/examples/qml_features/cxxqt/qml-features/cxx-qt-common/cxxqt_thread.h:32:10: error: 'shared_mutex' is unavailable: introduced in macOS 10.12
warning:   ::std::shared_mutex mutex;
warning:          ^
warning: /Applications/Xcode_13.2.1.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX12.1.sdk/usr/include/c++/v1/shared_mutex:180:58: note: 'shared_mutex' has been explicitly marked unavailable here
warning: class _LIBCPP_TYPE_VIS _LIBCPP_AVAILABILITY_SHARED_MUTEX shared_mutex
warning:                                                          ^

and

 warning: /Users/runner/cxx-qt/build/vcpkg_installed/x64-osx/include/Qt6/QtCore/qfileinfo.h:85:22: error: 'path' is unavailable: introduced in macOS 10.15
warning:     std::filesystem::path filesystemCanonicalFilePath() const
warning:                      ^
warning: /Applications/Xcode_13.2.1.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX12.1.sdk/usr/include/c++/v1/filesystem:902:24: note: 'path' has been explicitly marked unavailable here
warning: class _LIBCPP_TYPE_VIS path {
warning:                        ^

@BlackHoleFox
Copy link
Contributor

@ahayzen-kdab Do you have a MACOSX_DEPLOYMENT_TARGET set in your project? Based on those unavailable errors it seems like cxx-qt might have unintentionally been relying on the past behavior where cc wouldn't pass any deployment target to clang, so clang defaulted to the current OS (macOS 11 for you).

@twistedfall that's the same behavior cc used to have (rounding up the deployment target when using C++) but my recent PR made it a visible warning since it could be unexpected behavior for some projects/apps. You can get rid of it by picking a MACOSX_DEPLOYMENT_TARGET for your CI builds, and users won't see it via a crates.io download.

@ahayzen-kdab
Copy link

@BlackHoleFox Thanks you are correct adding MACOSX_DEPLOYMENT_TARGET: 10.15 to github actions has helped. Guess we need to find the correct spot to put this and whether it goes into our build scripts etc but that's off topic for here.

Look forward to 1.0.85 thanks!

@thomcc
Copy link
Member

thomcc commented Nov 14, 2023

Hmm, the goal with that PR was to avoid the breakage we might have when rustc bumps it's deployment target from 10.7 to 10.12, if it has the side effect of requiring the ecosystem to set their deployment targets, that's unintentional.

Most people will have no clue -- it's not like there's an easy way to map between C++ stdlib features and the version of macOS they were added.

This seems like a big and unintentional regression.

I'm away for the week (still working, but without access to an Intel mac, and with limited time for non-$dayjob work like cc), and I'm probably going to yank the latest release until we can get this sorted out.

Sorry for all the issues, everyone.

@thomcc
Copy link
Member

thomcc commented Nov 14, 2023

1.0.84 has been yanked.

@thomcc
Copy link
Member

thomcc commented Nov 14, 2023

Additionally it seems that it's running c++ --version a lot now

Ah, yeah, we should cache this result. Does that actually cause problems though? It's from a different change.

@twistedfall
Copy link
Author

Ah, yeah, we should cache this result. Does that actually cause problems though? It's from a different change.

Not really, it's just a new behavior that I noticed which seems suboptimal.

@thomcc
Copy link
Member

thomcc commented Nov 15, 2023

It should be cached.

@NobodyXu
Copy link
Collaborator

NobodyXu commented Nov 18, 2023

I will be working on caching the c++ --version, would submit a PR later today

Edit:

Opened #904 #932

@NobodyXu

This comment was marked as outdated.

@NobodyXu
Copy link
Collaborator

@thomcc Would it makes sense to bump the default MACOSX_DEPLOYMENT_TARGET to 10.15?

That might help reduce the regression introduced.

@Be-ing
Copy link
Contributor

Be-ing commented Jan 30, 2024

Thanks you are correct adding MACOSX_DEPLOYMENT_TARGET: 10.15 to github actions has helped. Guess we need to find the correct spot to put this and whether it goes into our build scripts etc but that's off topic for here.

Setting an environment variable isn't a great solution for downstream builders of crates that require deployment target minimums above cc's defaults. Unfortunately the existing Build::flag_if_supported doesn't quite work, so I think a new API is needed: #938

@NobodyXu
Copy link
Collaborator

NobodyXu commented Feb 4, 2024

hello @BlackHoleFox , I think we need to fix this regression and release a new version due to amount of commits/PRs unreleased.

However I'm not sure how to handle this issue, hence I'm asking for your help here.

@dpaoliello
Copy link
Contributor

Would it be possible to revert the commit that caused this issue to unblock a new release of cc-rs, then we can get that commit back in once we have a fix for this issue alongside it?

@BlackHoleFox
Copy link
Contributor

BlackHoleFox commented Feb 8, 2024

I mentioned this to @NobodyXu in a Zulip DM but I will be opening a PR shortly to revert the specific behavior change of querying rustc for the default version if nothing is supplied instead of using the SDKs' DefaultDeploymentTarget. I'll try and have that up tonight for review. This will bring most people back to the behavior before while still allowing people to use MACOSX_DEPLOYMENT_TARGET if they do set it.

@BlackHoleFox
Copy link
Contributor

The fixing PR is up now if anyone is interested in manually testing it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
7 participants