Remove sigvec reference in CMakeLists.txt #6958
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Thank you for supporting the project, and congratulations on your first contribution! A project committer will shortly review your contribution. In the mean time, if you haven't had a chance please skim over the contribution guidelines which all pull requests must adhere to. If the ECA pull request check fails, have a look at the instructions for signing the ECA in the legal considerations section.
If you run into any problems our community will be happy to assist you in any way we can. There are a number of recommended ways to interact with the community. We encourage you to ask questions, or drop by to say hello.
|
@FluffyFoxUwU
You need to sign the ECA. Please see the following instructions: https://github.com/eclipse/omr/blob/master/CONTRIBUTING.md#legal-considerations.
Instructions: https://github.com/eclipse/omr/blob/master/CONTRIBUTING.md#commit-guidelines
|
|
I'm not ready to give my real name in the Legal Name field yet. Is that mean I can't contribute until I give and sign it? |
|
@0xdaryl Do you have guidance for the above ECA question #6958 (comment)? |
|
@0xdaryl is on vacation. re #6958 (comment): @mstoodle Can @FluffyFoxUwU complete the ECA without providing a legal name? |
|
Hi @FluffyFoxUwU Notwithstanding that this particular PR only removes code (which will make part of this answer sound pretty ironic), I think you'll need to provide your legal name to the Eclipse Foundation to sign the ECA since you're basically asserting that you have the legal right to contribute these code changes and there needs to be a real person doing that (in case, say, there is ever a legal dispute over the provenance of the code contribution). I don't know whether your legal name becomes public knowledge as part of signing the ECA (I would be surprised, to be honest, but you would need to check that with the Eclipse Foundation), if that's your concern. As far as I know, the validation check just uses your Regardless of whether you end up deciding to sign the ECA, thanks for coming to the project and for trying to contribute an improvement. We appreciate that you volunteered your own time to the project! |
|
Lemme see if the Eclipse Foundation allows an exception for "removals" which would at least let us accept this PR. |
|
I sent a question to license@eclipse.org to ask if we can accept a code removal from an contributor who has not signed the ECA. If you want to follow up on the legal name requirement, you can also reach out to the same email address. I'll report back when I get a response. |
|
Ok, thanks. I'll wait for your response. |
|
I heard back from the EF and we are allowed to override the requirement if we are convinced that no IP is being added to the project, and I think this contribution meets this bar. I would like to see a committer approval including a statement that we've done all the relevant testing (@babsingh would you proceed with that, please?), then one of the leads (myself or @0xdaryl, if he's back, or even @charliegracie if he gets keen :) ) will merge this PR on that basis. Please @ mention the leads when this PR is ready to be merged. |
Yes, and I will ping the leads once the PR is ready to be merged. |
|
re #6958 (comment): @FluffyFoxUwU The commit message guidelines are still unsatisfied. The first line should be less than 70 chars. The subsequent lines (body) should be less than 72 chars. Please see the modified commit message below, which follows the guidelines. Also, the commit message and PR description should match. |
FoxieFlakey
force-pushed
the
patch-2
branch
2 times, most recently
from
469e890
to
c91481b
Compare
FoxieFlakey
changed the title
|
re #6958 (comment): @babsingh Ok, I fixed the commit message and PR message. And do I do the reply correctly? because this repo atleast has different way replying from what I can observe. |
|
@FluffyFoxUwU Yes, you replied correctly. Your code changes are good. Starting other testing: jenkins build all OSX failures are due to a known issue: #6516; so, they can be ignored. |
There was a problem hiding this comment.
@mstoodle @0xdaryl @charliegracie Testing looks good. No sigvec refs found in OMR or OpenJ9. So, it is safe to remove this code.
zOS PR build failed due to an infra issue. The code changes are disabled on zOS; so, we don't need a zOS PR build.
OSX failures are due to a known issue: #6516; so, they can be ignored.
|
jenkins build zos |
|
I have repushed to sign the commit. Please re-review the file changes @babsingh |
|
#6958 (comment) contains the corrected version of the commit message. The commit title on the first line is still missing: |
This was overlooked in eclipse#972 which removed all instances of sigvec. Fixes the below LLVM toolchain error: error: version script assignment of 'global' to symbol 'sigvec' failed: symbol not defined error when using LLVM toolchain
|
re #6958 (comment): @babsingh Oops, sorry I accidentally missed the title. |
|
OSX failure is #7181 (known and unrelated). The previous testing looked good. I have restarted the PR builds for reconfirmation. jenkins build all @0xdaryl As per #6958 (comment), can you please merge this PR once the PR builds pass? |
|
OSX PR build failed due to a known and unrelated issue: #7181. |
This was overlooked in #972 which removed all instances of sigvec.
Fixes the below LLVM toolchain error:
error: version script assignment of 'global' to symbol 'sigvec'
failed: symbol not defined error when using LLVM toolchain