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

[vcpkg] Add nologo to windows toolchain #11146

Merged
merged 4 commits into from
Jun 11, 2020
Merged

Conversation

Neumann-A
Copy link
Contributor

@Neumann-A Neumann-A commented May 3, 2020

closes #11144

There are still some linker logos in the log but i am apparently unable to find the correct cmake variable for it

@Neumann-A Neumann-A marked this pull request as ready for review May 4, 2020 22:06
@cenit
Copy link
Contributor

cenit commented May 5, 2020

just for symmetry reasons, do you maybe also need to add it to CMAKE_SHARED_LINKER_FLAGS_DEBUG and CMAKE_EXE_LINKER_FLAGS_DEBUG?

Also, why is there a /DEBUG flag in a *_RELEASE symbol? I realize it was already there, just questioning it in principle

@LilyWangL LilyWangL self-requested a review May 6, 2020 01:59
@LilyWangL LilyWangL self-assigned this May 6, 2020
@LilyWangL LilyWangL changed the title Add nologo to windows toolchain [vcpkg] Add nologo to windows toolchain May 6, 2020
@LilyWangL LilyWangL requested a review from strega-nil May 6, 2020 06:42
ports/fftw3/CONTROL Outdated Show resolved Hide resolved
@BillyONeal
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@strega-nil
Copy link
Contributor

@Neumann-A why do you have two different styles of adding this flag? both set and string(APPEND? Is there a reason for it?

@Neumann-A
Copy link
Contributor Author

@Neumann-A why do you have two different styles of adding this flag? both set and string(APPEND? Is there a reason for it?

_INIT flags versus normal flags. The set command completely overwrites the other variables and _INIT flags are used to initialize the normal values. I am just following the style of the other toolchains which use _INIT with string(APPEND)

if(NOT _CMAKE_IN_TRY_COMPILE)
string(APPEND CMAKE_C_FLAGS_INIT " -fPIC ${VCPKG_C_FLAGS} ")
string(APPEND CMAKE_CXX_FLAGS_INIT " -fPIC ${VCPKG_CXX_FLAGS} ")
string(APPEND CMAKE_C_FLAGS_DEBUG_INIT " ${VCPKG_C_FLAGS_DEBUG} ")
string(APPEND CMAKE_CXX_FLAGS_DEBUG_INIT " ${VCPKG_CXX_FLAGS_DEBUG} ")
string(APPEND CMAKE_C_FLAGS_RELEASE_INIT " ${VCPKG_C_FLAGS_RELEASE} ")
string(APPEND CMAKE_CXX_FLAGS_RELEASE_INIT " ${VCPKG_CXX_FLAGS_RELEASE} ")
string(APPEND CMAKE_SHARED_LINKER_FLAGS_INIT " ${VCPKG_LINKER_FLAGS} ")
string(APPEND CMAKE_EXE_LINKER_FLAGS_INIT " ${VCPKG_LINKER_FLAGS} ")
if(VCPKG_CRT_LINKAGE STREQUAL "static")

@strega-nil
Copy link
Contributor

Ah, I see! why not modify the CMAKE_EXE_LINKER_FLAGS_RELEASE_INIT and CMAKE_SHARED_LINKER_FLAGS_RELEASE_INIT then?

@Neumann-A
Copy link
Contributor Author

Because it would require me to check the CMake source again to check how to change

    set(CMAKE_SHARED_LINKER_FLAGS_RELEASE "/nologo /DEBUG /INCREMENTAL:NO /OPT:REF /OPT:ICF ${VCPKG_LINKER_FLAGS}" CACHE STRING "")
    set(CMAKE_EXE_LINKER_FLAGS_RELEASE "/nologo /DEBUG /INCREMENTAL:NO /OPT:REF /OPT:ICF ${VCPKG_LINKER_FLAGS}" CACHE STRING "")

into the CMAKE_EXE_LINKER_FLAGS_RELEASE_INIT|CMAKE_SHARED_LINKER_FLAGS_RELEASE_INIT variables if this is even possible. Since the last time I changed those variables I broke vcpkg (da0a154 9b44e47) I don't dare to touch them again for some time ;)

@cenit
Copy link
Contributor

cenit commented May 8, 2020

I'd trigger a full rebuild before merging, just in case ;)

@cenit
Copy link
Contributor

cenit commented May 8, 2020

@strega-nil (something that a toolchain modification should already do 😉)

@dan-shaw dan-shaw merged commit c704058 into microsoft:master Jun 11, 2020
JangBoo pushed a commit to JangBoo/vcpkg that referenced this pull request Jun 18, 2020
* add nologo to windows toolchain

* bump control of one cmake port for ci testing

* [scripts] add nologo to _DEBUG flags

* revert version bump
@Neumann-A Neumann-A deleted the add_nologo branch July 17, 2020 19:26
penumbra23 pushed a commit to codespace-dev/vcpkg that referenced this pull request Aug 5, 2020
* add nologo to windows toolchain

* bump control of one cmake port for ci testing

* [scripts] add nologo to _DEBUG flags

* revert version bump
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.

[vcpkg] Add /nologo to build flags on windows
6 participants