-
Notifications
You must be signed in to change notification settings - Fork 386
Remove the long-deprecated Makefile build #320
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
base: main
Are you sure you want to change the base?
Conversation
There used to be a Makefile-based implementation of this test suite. However, it has been documented as deprecated for at least 13 years, it is unmaintained and it most likely doesn't work anymore. This patch removes Makefiles throughout the code base and updates the documentation. In addition to cleaning up the llvm-test-suite, this removal will make it possible to remove the corresponding lnt test plugin that we otherwise have to maintain in lnt.
|
I'm not certain how to best go about doing this. I'm not intimately familiar with how the LLVM test suite is used and whether some people might in fact still be using Makefile support even though it's officially deprecated. Is there someone that would have that high level understanding? |
|
I remember in the past there were usually some people popping up saying they used the makefile based system when someone proposed to remove it... (sounded more like in-house systems that are not visible/known to the general LLVM public community); though I believe there is also still a makefile based mode left in the LNT runner (but LNT can happily run cmake and I hope there is no usage of the makefile based mode left). That said I think it was always silly to maintain both for as long as we did, and I regret that we did not set any deadlines or official deprecation notices and just carried on (that was also my fault)... Maybe put up an RFC on the discourse and hopefully/maybe there is no real user left? Or if there are users left maybe they are fine staying on an old snapshot version of llvm-test-suite (we could make a link / git tag for it)... |
|
I suspect the whole |
|
In fact the tests in the |
jerryzj
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested locally on our downstream CI - the CMake build works fine after this change.
However, as @MatzeB mentioned, there have historically been users relying on the Makefile build. I'd suggest posting an RFC on LLVM Discourse to give the community a heads-up and a chance to voice any concerns before merging.
|
I'm supportive of removing this. A heads-up / RFC on Discourse seems sensible in case we're surprised to find users. But if they do exist I suspect they'd be equally well served by checking out an older version of the repo or reverting the patch locally given the Makefiles seem totally unmaintained. |
fhahn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm supportive of removing this. A heads-up / RFC on Discourse seems sensible in case we're surprised to find users. But if they do exist I suspect they'd be equally well served by checking out an older version of the repo or reverting the patch locally given the Makefiles seem totally unmaintained.
Removing them sounds like a great clean up to me! I agree that a heads-up on discourse would be nice, as @asb suggested
|
Ack folks, thanks a lot for chiming in. I'll write a RFC and post it on Discourse.
Yes, in fact removing that is how I ended up creating this PR! |
|
Discourse RFC is here: https://discourse.llvm.org/t/llvm-test-suite-removing-the-deprecated-makefiles! |
|
LGTM |
There used to be a Makefile-based implementation of this test suite. However, it has been documented as deprecated for at least 13 years, it seems to be unmaintained and it most likely doesn't work anymore.
This patch removes Makefiles throughout the code base and updates the documentation.
In addition to cleaning up the llvm-test-suite, this removal will make it possible to remove the corresponding lnt test plugin that we otherwise have to maintain in lnt.