On Oct 25, 2012, at 9:39 PM, Magnus Ihse Bursie wrote: > As I said, the name ALT_OUTPUTDIR is already used in jdk/test/Makefile. That > is the reason I used it. Of course it could be called anything, but if it is > not called the same thing across repos, we get a hard time for no good reason.
I originally had visions of the test makefiles sharing logic with the build makefiles, then thought better of it. There are advantages to keeping the test makefile logic separate, or if not separate, carefully sharing select logic. My attitude now is that separate is better, and we should avoid any possible variable name collisions to avoid confusion and prevent any future sharing tricks. Testing configurations turn out to be different than build configurations. This test/Makefile logic started out extremely simple, got very complicated, then Jonathan started improving jtreg to support some of the issues we had, and then test/Makefile started getting a bit more simple again. So this file has been evolving quite a bit. At some point I decided that this was the best way for JPRT to run tests because it allowed the developer to use the same test targets for testing as JPRT, and insulated JPRT from having to know how the test harnesses worked. > > The only important thing for integration with build-infra is that the > langtools test makefile gets a customizable output directory, so it is not > hard-coded to the old output directory. > Fair enough, but let's see if we can integrate that change into the jdk8/tl/langtools repository. Jonathan Gibbons should look at the change if he has the time. > The build-infra test integration currently calls test/Makefile in the root, > and that works fine. I do not remeber if it is because it already has this > variable, or because it does not output anything, but I suspect thr latter. > > I was not aware that the hotspot repo also shared this test logic, so I > haven't checked there. "sharing" is probably not the right word, "copied" would be better, unfortunately. :^( If we could put some shared test make logic in the top level common/makefiles area, we could try and share that, but hotspot still has completely independent makefiles, so some copies might be needed for now, in hotspot. -kto > > /Magnus > > 25 okt 2012 kl. 22:40 skrev Kelly O'Hair <kelly.oh...@oracle.com>: > >> >> This change is not right. >> >> We should not use the name ALT_ anything in this makefile, it is an >> independent Makefile. >> For better or worse, it does not share the build make logic at all. >> >> The names ALT_OUTPUTDIR and ABS_OUTPUTDIR could just be something like >> TEST_OUTPUT_ROOT >> or something without the ALT or OUTPUTDIR names. >> >> If these var names were changed I'd be fine with this basic change. But at >> the same time, there are also the test/Makefiles >> in jdk, hotspot, and the root. I would think we should at least verify those >> don't need the same change. >> >> -kto >> >> On Oct 25, 2012, at 4:42 AM, Magnus Ihse Bursie wrote: >> >>> There are several similarities between jdk/test/Makefile and >>> langtools/test/Makefile. This makes it possible to run tests from either >>> one in build-infra (the new build system) by using the target "test". In >>> jdk/test/Makefile there is a test for an external variable ALT_OUTPUTDIR, >>> which directs where the test results should be stored. The build-infra >>> framework uses this to keep the test output in the corresponding build >>> directory. >>> >>> However, while the code is very similar in langtools/test/Makefile, this >>> exact feature is not available. This results in langtools tests being >>> hardcoded to end up in a directory named as in the old build system. >>> >>> Here is a rather trivial patch that will add the ALT_OUTPUTDIR >>> functionality to langtools as well. >>> >>> http://cr.openjdk.java.net/~ihse/support-build-infra-output-in-langtools-test/webrev.00/ >>> >>> I'm assuming someone in the langtools group should review this as well, and >>> that it probably should go in via the -tl forest. I couldn't figure out >>> what mailing list to reach the langtools group on though, so I'd >>> appreaciate some help with forwarding this to the right persons. >>> >>> /Magnus >>
