| Summary: | Out of tree build | ||
|---|---|---|---|
| Product: | [Developer tools] valgrind | Reporter: | Gilles Chanteperdrix <gilles.chanteperdrix> |
| Component: | general | Assignee: | Julian Seward <jseward> |
| Status: | CONFIRMED --- | ||
| Severity: | wishlist | CC: | flo2030, gerickson, mark, philippe.waroquiers |
| Priority: | NOR | ||
| Version First Reported In: | unspecified | ||
| Target Milestone: | --- | ||
| Platform: | unspecified | ||
| OS: | Linux | ||
| Latest Commit: | Version Fixed In: | ||
| Sentry Crash Report: | |||
| Attachments: |
patch
valgrind-out-of-tree-regtest.diff patch to get make check to work patch version 2 to get make check to succeed |
||
Thanks. Building with builddir != srcdir is certainly useful. There was one change in Makefile.tool-tests.am that was not complete/correct: AM_CPPFLAGS = -I$(top_srcdir) -I$(top_srcdir)/include \ -I$(top_srcdir)/coregrind -I$(top_builddir)/include \ - -I$(top_srcdir)/VEX/pub \ + -I$(top_builddir)/VEX/pub \ You want to also include the $(top_srcdir)/VEX/pub. I pushed your patch with that added back. And without the README update. Since there is more to do than just this. make && make check now works for builddir != srcdir, but make regtest doesn't yet. For that we probably have to hack the perl script a little to use the binaries from the builddir but the vgtest and stdout/err files from the srcdir. If we fix that, then we can recommend people build that way in the README. Committed r13949. On 05/09/2014 03:35 PM, Mark Wielaard wrote: > https://bugs.kde.org/show_bug.cgi?id=333628 > > Mark Wielaard <mjw@redhat.com> changed: > > What |Removed |Added > ---------------------------------------------------------------------------- > Status|UNCONFIRMED |CONFIRMED > Ever confirmed|0 |1 > > --- Comment #1 from Mark Wielaard <mjw@redhat.com> --- > Thanks. Building with builddir != srcdir is certainly useful. There was one > change in Makefile.tool-tests.am that was not complete/correct: > > AM_CPPFLAGS = -I$(top_srcdir) -I$(top_srcdir)/include \ > -I$(top_srcdir)/coregrind -I$(top_builddir)/include \ > - -I$(top_srcdir)/VEX/pub \ > + -I$(top_builddir)/VEX/pub \ > > You want to also include the $(top_srcdir)/VEX/pub. I pushed your patch with > that added back. And without the README update. Since there is more to do than > just this. make && make check now works for builddir != srcdir, but make > regtest doesn't yet. For that we probably have to hack the perl script a little > to use the binaries from the builddir but the vgtest and stdout/err files from > the srcdir. If we fix that, then we can recommend people build that way in the > README. > > Committed r13949. > Ok, thanks. Note that you can probably also close bug 256174.
> You want to also include the $(top_srcdir)/VEX/pub. I pushed your patch with
> that added back. And without the README update. Since there is more to do
> than just this. make && make check now works for builddir != srcdir, but
> make regtest doesn't yet. For that we probably have to hack the perl script
> a little to use the binaries from the builddir but the vgtest and stdout/err
> files from the srcdir.
Gilles, thank you very much for your patch.
Is there any possibility you could look at the out-of-tree 'make
regtest' case? It would be really useful if that worked, and the lack
of it somewhat undermines the usefulness of out-of-tree builds.
*** Bug 256174 has been marked as a duplicate of this bug. *** Created attachment 86550 [details] valgrind-out-of-tree-regtest.diff On 05/09/2014 06:09 PM, Julian Seward wrote: > Gilles, thank you very much for your patch. > > Is there any possibility you could look at the out-of-tree 'make > regtest' case? It would be really useful if that worked, and the lack > of it somewhat undermines the usefulness of out-of-tree builds. I have tried, but I am afraid I do not know enough the valgrind project (and perl) to be able to continue. Please find attached my attempt. Running "make regtest" hangs when running the first test, my guess would be that the program input is not correctly redirected from the .stdin file in the sources directory, but I do not understand why. Thanks for trying. Looking at tests/vg_regtest.in it does indeed seem somewhat hard to teach it about split source/build dir. It relies on chdir and the cwd a lot. We'll need to audit all the system (and mysystem) calls to see whether the cwd, the input and output files/redirects are correct. The vgtest diff/filter calls seem to depend on the cwd being the (sub)srcdir of the test, while the prog, prereq, post, etc vgtest calls seem to depend on the cwd being the (sub)builddir of the test. I think the approach I took is the wrong one: it seems best to recurse in the source tree, and only use the build tree to look for compiled programs, and put the output files. Everything else is in the source tree. Created attachment 86666 [details]
patch to get make check to work
make check does not work out of tree, because compiling exp-bbv/tests/amd64-linux/ll.S fails.
The reason is that that file uses
.include "../logo.include"
and that file does not exist out of tree.
Now, the gcc invocation has all the necessary -I directives, but those aren't passed down to the assembler unless prefixed by -Xassembler.
The patch fixes this, but it is not particularly elegant. Adding -Xassembler should ideally be done elsewhere where the -I stuff is pieced together. But I could not figure out where that happens. The directive we need to pass to the assembler is -I.........../trunk/exp-bbv/tests/amd64-linux etc
(In reply to comment #8) > The patch fixes this, but it is not particularly elegant. Thanks for looking into this. re the ll.S problem, is it viable to instead pass on the gcc invokation for ll.S, something like -DTHEPATHYOUNEED=../../../whatever and have in ll.S .include $THEPATHYOUNEED kind-of-kludge? (In reply to comment #9) > (In reply to comment #8) > > The patch fixes this, but it is not particularly elegant. > > Thanks for looking into this. > > re the ll.S problem, is it viable to instead pass on the gcc invokation for > ll.S, > something like > -DTHEPATHYOUNEED=../../../whatever > > and have in ll.S > > .include $THEPATHYOUNEED > > kind-of-kludge? Ugly, too. You need double quoting in the Makefile -I'"..."' and you cannot pass -D to AM_CCASFLAGS .. What I had hoped for was to change exp-bbv/tests/Makefile.am and modify AM_CCASFLAGS there (i.e. in a siongle spot) as opposed to change the makefiles in all subdirs. But that did not work and I don't have the skills (and not the nerve either :) to debug this. So I'm proposing to check in the patch below which differs from the previous one in that it removes the relative path from the include directives in the .S files. That allows to use the same -I... option in all makefiles. Created attachment 86669 [details]
patch version 2 to get make check to succeed
(In reply to comment #11) > Created attachment 86669 [details] > patch version 2 to get make check to succeed Took a quick look at it. With my (as bad or worse than yours) auto.... skills, the patch looks ok to me. Mark, does the patch looks sane to you ? (we still need a small addition in README, to explain how to make out of tree build) (In reply to comment #12) > (In reply to comment #11) > > Created attachment 86669 [details] > > patch version 2 to get make check to succeed > > Took a quick look at it. With my (as bad or worse than yours) auto.... > skills, > the patch looks ok to me. I checked this is a while ago as r13982. Looked a bit at getting make regtest to work. A quick and dirty approach would perhaps be to simply copy the expected files (.exp .vgtest etc files) into the corresponding out-of-tree locations. That isn't pretty but could probably be done if that apparoach is agreeable. One could possibly fix up vgtest along the lines Mark suggested if one wanted to. But the time doing so would be better spent imho to integrate the regtesting into the make machinery such that tests can be run in parallel -- something that vgtest won't let me do today. |
Created attachment 86174 [details] patch Autotools allow packages using them to be built out of their source tree. It would be nice if valgrind could be built that way, as it allows the same source tree to be used for many builds with differing configure options, compilation flags, or for different architectures. Please find attached a patch which adds this functionality to valgrind current trunk (revision 13899).