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

check_linker_flag: use for linker flags #1525

Merged
merged 1 commit into from Jul 15, 2020

Conversation

grooverdan
Copy link
Member

@grooverdan grooverdan commented May 5, 2020

Fixes noisy clang output like #1524

Treats "-Wl,-z,relro,-z,now" as an actual linker flags rather than compile flags.

Slightly WIP due to being a purely linker check rather than testing module and shared library builds separately, however it does have an advantage over #1524 in that the linker flags are still used in clang.

# TODO - SET(HAVE_SHARED..) is temporary only. remove once
# MY_CHECK_SHARED_LINKER_FLAG perfoms independent check.
SET(HAVE_SHARED_${result} HAVE_MODULE_${result})
FOREACH(linktype SHARED MODULE)
Copy link
Contributor

Choose a reason for hiding this comment

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

What about CMAKE_EXE_LINKER_FLAGS? Do you think it's not needed here?

Copy link
Member Author

@grooverdan grooverdan May 5, 2020

Choose a reason for hiding this comment

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

Good call, it is needed.

#The regex patterns above are not localized, thus LANG=C
SET(ENV{LANG} C)

MACRO (MY_CHECK_MODULE_LINKER_FLAG flag)
Copy link
Member

Choose a reason for hiding this comment

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

Why "MODULE", How does it check specifically for MODULE?

Copy link
Member Author

Choose a reason for hiding this comment

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

This was the WIP bit, waiting for a clue to see how to test module vs shared library vs exe . I couldn't immediately see one, so was keeping it separate for now. If your happy with just this implementation I can rename/merge it with MY_CHECK_LINKER_FLAG.

ENDMACRO()

FUNCTION(MY_CHECK_AND_SET_LINKER_FLAG flag_to_set)
# At the moment this is gcc-only.
Copy link
Member

Choose a reason for hiding this comment

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

not really "gcc-only", is it? 😄 just remove this line

FAIL_REGEX "warning:.*ignored"
FAIL_REGEX "warning:.*is valid for.*but not for"
FAIL_REGEX "warning:.*redefined"
FAIL_REGEX "[Ww]arning: [Oo]ption"
Copy link
Member

Choose a reason for hiding this comment

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

Not sure you need all these patterns here

Copy link
Member Author

Choose a reason for hiding this comment

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

nope, but I've no idea how linkers would issue warnings and these seem to be reasonable texts for now.

-Wl,-z,relro,-z,now are linker flags and should
be checked as such.

TODO: perform module, exe shared checks separately
rather than a pure linker check.
@grooverdan grooverdan force-pushed the 10.1-WIP-check-and-set-linker-flag branch from f4d0a9f to fb1d30c Compare May 5, 2020 23:49
@grooverdan
Copy link
Member Author

grooverdan commented May 5, 2020

Ready now.

In the absence of a clear way to separate module, shared, exe tests, just assumed the linker can take the same flags in all scenarios. So this has been simplified down. fail regexs removed until a case is needed, and added the EXE_FLAGS.

@an3l an3l added this to the 10.1 milestone May 7, 2020
@an3l an3l added the license-mca Contributed under the MCA label May 7, 2020
@grooverdan
Copy link
Member Author

Well no-one's come up with a good module/linker/exe test separately (https://stackoverflow.com/questions/61604663/how-do-i-test-cmake-shared-linker-flags-and-cmake-module-linker-flags), so is thi ok @vuvova ?

@kevgs kevgs merged commit 7473e18 into MariaDB:10.1 Jul 15, 2020
@kevgs
Copy link
Contributor

kevgs commented Jul 15, 2020

Thanks for the patch!

@grooverdan
Copy link
Member Author

Thanks for the merge.

@grooverdan grooverdan deleted the 10.1-WIP-check-and-set-linker-flag branch July 15, 2020 06:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
license-mca Contributed under the MCA
4 participants