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.

Sign up for GitHub

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

Jump to bottom

[C++] Add mold linker flags to the CMake linker flags variables #40602

Closed
cryos opened this issue Mar 16, 2024 · 1 comment
Closed

[C++] Add mold linker flags to the CMake linker flags variables #40602

cryos opened this issue Mar 16, 2024 · 1 comment
Assignees
@cryos
Labels
Component: C++ Type: bug
Milestone
16.0.0

Comments

@cryos
Copy link
Contributor

cryos commented Mar 16, 2024

Describe the bug, including details regarding any error messages, version, and platform.

I noticed the recent support for the mold linker, it currently adds the flags to the CMake compiler flags for C and C++ whereas it would be better to do this using the linker flags. I was going to create a PR to do that and this issue accompanies it.

This leads to less invalidation of cache when switching linkers as only the links would use the flag and is the more correct way to manipulate linker flags.

Component(s)

C++
cryos added a commit to cryos/arrow that referenced this issue Mar 16, 2024
@cryos
apacheGH-40602: Move mold linker flags to variables
8e5f111 
Move the mold linker flags to the CMake CMAKE_*_LINKER_FLAGS variables.
@github-actions github-actions bot mentioned this issue Mar 16, 2024
Merged
kou added a commit that referenced this issue Mar 19, 2024
@cryos @kou
GH-40602: [C++] Move mold linker flags to variables (#40603)
bb9f229 
### Rationale for this change

Move the mold linker flags to the CMake `CMAKE_*_LINKER_FLAGS` variables. This is the more correct place to add them as they will only be used during linking, it would invalidate any [s]ccache less frequently if changing linkers and I think it is more clear/correct.

### What changes are included in this PR?

Moving the mold linker flags from the `CMAKE_*_FLAGS` to `CMAKE_*_LINKER_FLAGS`.

### Are these changes tested?

Existing tests and validation would cover these changes and the result is essentially the same as before.

### Are there any user-facing changes?

No
* GitHub Issue: #40602

Lead-authored-by: Marcus D. Hanwell <marcus@voltrondata.com>
Co-authored-by: Marcus D. Hanwell <mhanwell@gmail.com>
Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
@kou kou added this to the 16.0.0 milestone Mar 19, 2024
@kou
Copy link
Member

kou commented Mar 19, 2024

Issue resolved by pull request 40603
#40603

@kou kou closed this as completed Mar 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@cryos cryos

Labels
Component: C++ Type: bug
Projects
None yet
Milestone
16.0.0
Development

No branches or pull requests
2 participants
@kou @cryos