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

Patch allow gcc10 #1010

Merged
merged 4 commits into from
Jun 17, 2020
Merged

Patch allow gcc10 #1010

merged 4 commits into from
Jun 17, 2020

Conversation

j4qfrost
Copy link
Contributor

Resolves this issue neovide/neovide#306

@j4qfrost
Copy link
Contributor Author

I haven't had any luck creating a good testing environment with gcc10 with github actions. Anyone want to take a stab at that?

@Cobrand
Copy link
Member

Cobrand commented Jun 16, 2020

if local_ver >= affected_ver {
    cfg.cflag("-fcommon");
    cfg.cflag("-fPIE");
}

Why is this necessary? It seems to be the core of the fix, but we don't know why those two flags are necessary.

For github actions, I have no clue. We were using travis at first, but it broke at some point for some reason, so we need to merge the travis script to github actions or vice-versa, so in the meanwhile it's not really that important I guess.

@j4qfrost
Copy link
Contributor Author

j4qfrost commented Jun 16, 2020

I'm not totally sure about why adding those flags works, but a number of users for a text editor that I'm supporting ran into a linking issue on systems using gcc 10. I found the suggested fix here onivim/oni2#1778 and have tested adding the cflags locally, and it fixes my issue. Alternatively, the issue can be fixed by simply doing CFLAGS='-fcommon -fPIE' cargo run, so this change isn't entirely necessary.

@Cobrand
Copy link
Member

Cobrand commented Jun 17, 2020

Can you test with only -fcommon without -fPIE? PIE is for Position Independant Executable, and I'm not sure it's useful here (I read that if fixes some issues with dlopen, but we're not using it here so...)

@j4qfrost
Copy link
Contributor Author

I checked and it works. I think the PIE requirement might be on our side on specific machines.

@Cobrand
Copy link
Member

Cobrand commented Jun 17, 2020

Thanks!

@Cobrand Cobrand merged commit 92fb34d into Rust-SDL2:master Jun 17, 2020
ttys3 added a commit to ttys3/skulpin that referenced this pull request Aug 29, 2020
ttys3 added a commit to ttys3/skulpin that referenced this pull request Aug 29, 2020
sypwex pushed a commit to sypwex/rust-sdl2 that referenced this pull request Jun 2, 2024
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.

2 participants