This is an archive of the discontinued LLVM Phabricator instance.

Remove autogenerated makefiles after running tests
AbandonedPublic

Authored by labath on Feb 4 2015, 6:46 AM.

Details

Reviewers
vharron
clayborg
Summary

Running the test suite leaves about 10 autogenerated Makefiles in the test tree. These files
clobber "git status" output and make general workflow difficult. This commit makes that the
autogenerated Makefiles are deleted after the tests run (unless LLDB_DO_CLEANUP=NO). It also
removes from svn two autogenerated makefiles which have been commited in the past.

Diff Detail

Event Timeline

labath updated this revision to Diff 19316.Feb 4 2015, 6:46 AM
labath retitled this revision from to Remove autogenerated makefiles after running tests.
labath updated this object.
labath edited the test plan for this revision. (Show Details)
labath added reviewers: clayborg, vharron.
labath added a subscriber: Unknown Object (MLST).
vharron edited edge metadata.Feb 4 2015, 7:01 AM

Can you add the auto generated Make files to .gitignore / svn ignore (with
comment!) so they won't be re-added?

labath added a comment.Feb 4 2015, 7:21 AM

I'm particularly fond of that idea. Even if I put full paths to all 10 makefiles to .gitignore now (and I need to put full paths, as we don't want to ignore all makefiles), the list will get quickly out of sync as test cases are added/removed.

Besides, if this patch goes in in this form, adding these makefiles back will be pretty difficult as they will get deleted after every testsuite run.

zturner added a subscriber: zturner.Feb 4 2015, 7:49 AM

+1 for not touching gitignore. Seems like a bandaid for the real problem.
Tests should be smart enough to clean up after themselves.

I thought there were cases where a Makefile was checked into tree. This
would also delete those right?

labath added a comment.Feb 4 2015, 7:59 AM

If the tests are interrupted, they will leave a lot more files behind than just Makefiles, so you cannot possibly .gitignore all of them. This would need a much more principled approach.

@zturner: yes, this commit deletes the (only?) two in-tree Makefiles, which can be autogenerated by the test suite. Note that there are a lot of Makefiles in the test tree which cannot be/are not autogenerated.

If it deletes the two that are checked in, those will still show up as
changes (deletes) to git, so it just shifts the problem

Ahh i see, you've deleted the two makefiles. I'm pretty sure that's a
blocker, i tried this before and was stopped by Greg and Jim

labath added a comment.Feb 4 2015, 8:13 AM

I'm all for having temporary files out of the source tree. It is not going to be easy though...

Yes, i delete the makefiles from the tree. Do you know what was the reason they did not like that.

I saw that Greg put one of these files in, that's why he's on cc: :)

I guess the reason is something like, being able to run the test manually if it is failing. However, I would say that in this case you should run the test with LLDB_DO_CLEANUP=NO and that will leave all the temporaries (including the Makefile) for analysis. I think the common case (running the testsuite without failures) should be as painless as possible and in case you really need to debug, rerunning a single test is not a big deal.

Here's the original thread:
http://lists.cs.uiuc.edu/pipermail/lldb-dev/2014-December/006117.html

I would suggest starting a new thread on lldb-dev and linking to that post
for some context, and explaining the current state of things and your
proposed solution.

clayborg requested changes to this revision.Feb 4 2015, 10:57 AM
clayborg edited edge metadata.

Please just check the Makefile files in, don't remove them. Why? Because when this test fails, I want to be able to go to each directory and type "make" and then be able to debug it. If there are no makefiles, you can only run it by running the inline test. This is why the Makefiles were left there. So just run these tests once and check in any Makefile files that are not already checked in.

This revision now requires changes to proceed.Feb 4 2015, 10:57 AM

Right, so now the issue is that the test runner tries to delete a file
which is checked in, causing git to detect changes. Do we change the test
runner to never delete generated Makefiles? It seems like there was some
contention about that last time I ran into this problem. It's all very
flaky right now, because the generated filename is the same as the checked
in filename. We should make a decision, probably from the following list:

  1. The test runner will always generate Makefiles, and Makefiles will never

be checked in

  1. The test runner will never generate Makefiles, and Makefiles will always

be checked in

  1. The test runner will generate Makefiles whenever it wants, but with

*different
names* than those of the files that are checked in.

I'm going to solve this problem locally by modifying my .git/info/exclude and/or core.excludesfile

Note that since some of the Makesfiles actually are checked in, you will be
ignoring some versioned files. It's probably less of an inconvenience than
what you're currently experiencing, but just an fyi.

Greg, I thought about this some more and, i think *for now*, we need to at
the very least rename these Makefiles in the repo.

As it stands, the test runner is broken in the presence of checked in
Makefiles, and it's severely annoying for those of us who are affected by
this.

You can still debug it this way, instead of typing make you just run make
with whatever option lets you specify the input makefile.

E.g. "make -f Makefile.local"

labath abandoned this revision.Feb 5 2015, 9:08 AM

I have checked in the generated makefiles, per @clayborg's suggestion.

This revision is abandoned.