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

Do not mark "removing unused section" lines as errors #19

Merged
merged 1 commit into from
Aug 22, 2022

Conversation

githubaf
Copy link
Contributor

gcc linker outputs "removing unused section" to stderr when the option "--print-gc-sections" is used. That is fine.

Eclipse CDT reports all these lines as errors however and the build seems to have failed, which is wrong.
The was a bug report Bug 539927: Do not mark "Removing unused section" lines as errors and a fix
To fix this bug a change was made 2020-12-02 0532265
-CDTGNULinkerErrorParser.regex.LdMode=(.[/\\])?ld(\.exe)?: (mode .)
+CDTGNULinkerErrorParser.regex.ldInfo=(.[/\\])?ld(\.exe)?: ((mode|Removing unused section) .)

That does however not solve problem entirely, as the output of gcc linker is "removing", not "Removing", i.e. the word "removing" starts with a lower-case "r"
To fix that problem both R and r should be accepted.

…n "--print-gc-sections" is used. That is fine.

Eclipse CDT reports all these lines as errors however and the build seems to have failed, which is wrong.
The was a bug report Bug 539927: Do not mark "Removing unused section" lines as errors and a fix
To fix this bug a change was made 2020-12-02 0532265
-CDTGNULinkerErrorParser.regex.LdMode=(.*[/\\\\])?ld(\\.exe)?: (mode .*)
+CDTGNULinkerErrorParser.regex.ldInfo=(.*[/\\\\])?ld(\\.exe)?: ((mode|Removing unused section) .*)

That does however not solve problem entirely, as the output of gcc linker is "removing", not "Removing", i.e. the word "removing" starts with a lower-case "r"
To fix that problem both R and r should be accepted.
@jonahgraham
Copy link
Member

Sorry @githubaf - this repo (for now) is just a mirror of CDT. However we are just in the process of migrating to GitHub so perhaps this will be the first GitHub PR that gets merged once that migration is done!

In the meantime, you need to sign the Eclipse Contributor Agreement so that we can accept any changes. You can review and sign the agreement here https://accounts.eclipse.org/user/eca

@githubaf
Copy link
Contributor Author

OK, Thanks for the information. I did.

@cmorty
Copy link
Contributor

cmorty commented Aug 16, 2022

@githubaf Merging is open -> This needs rebasing.

@jonahgraham
Copy link
Member

From the code perspective, this change is ready for committing - no need to rebase it, the continuous-integration/jenkins/pr-merge check shows two unrelated test failures when this change is merged to main branch.

However the ECA check continues to fail. Please ensure that your commit author email address matches that with which you signed the ECA with.

I know that it is quite a bit of process for a trivial commit, but it is a one time thing and once done we can accept your contributions.

@jonahgraham
Copy link
Member

PS1 When this change is eventually merged, it will be merged using the "rebase and merge" or "squash and merge" strategy on GitHub. Since we test PRs on Jenkins by merging this change with current main, we know it is good to go.

PS2 We are still learning and adapting to our change to GitHub. The reporting back from Jenkins of checks failing needs improving. Some flaky tests still need to be marked as so with marking tests as flaky. PRs on that topic welcome.

@githubaf
Copy link
Contributor Author

Seems the email address I used in the ECA is a different (but correct one) from the github account. What should I do now? Update the ECA on https://accounts.eclipse.org?
I am sorry for the inconvenience.

@jonahgraham
Copy link
Member

I apologize for the inconvenience. You are not the first person to have this type of problem. Please try to update your eclipse account with the same email address in your commit message. If you have trouble please raise a ticket on the helpdesk or comment here and we'll try to resolve the issue.

@jonahgraham jonahgraham added this to the 11.0.0 milestone Aug 21, 2022
@jonahgraham
Copy link
Member

Thanks @githubaf for persisting. The ECA check now passes. I will merge this once I resolve #55.

@githubaf
Copy link
Contributor Author

Sounds good! Thank you!

@jonahgraham jonahgraham merged commit 9ce74a7 into eclipse-cdt:main Aug 22, 2022
@jonahgraham
Copy link
Member

Thank you @githubaf for the contribution.

@jonahgraham jonahgraham added the noteworthy Pull requests and fixed issues that should be highlighted to users label Sep 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
noteworthy Pull requests and fixed issues that should be highlighted to users
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants