Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#12640 closed defect (fixed)

Entry guard unittest (#12207) fail in out-of-tree builds

Reported by: asn Owned by:
Priority: Medium Milestone: Tor: 0.2.5.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: tor-guard
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

In the unittests introduced by #12207, I'm reading a file containing some fake descriptors.

The file is at /src/test/test_descriptors.txt and when I'm loading it in the code, I do:

static void *
fake_network_setup(const struct testcase_t *testcase)
{
  /* This is the file containing our test descriptors. */
  const char *fname = BUILDDIR "/src/test/test_descriptors.txt";

But apparently, the BUILDDIR macro was completely wrong there, since I was looking for the root of the source directory, whereas BUILDDIR is the directory we are building (who would have thought). Apparently, this breaks the unittests in the out-of-tree builds of jenkins.

We should probably find the right macro to use there. I didn't find a SRCDIR in config.log but maybe autotools already have something for us?

Child Tickets

Attachments (1)

0001-Make-out-of-tree-builds-work-again.patch (1.5 KB) - added by weasel 5 years ago.
I like this version slightly better.

Download all attachments as: .zip

Change History (6)

comment:1 Changed 5 years ago by weasel

This makes it build once more:

diff --git a/configure.ac b/configure.ac
index 414c72a..20e91d0 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1395,6 +1395,13 @@ AC_SUBST(BUILDDIR)
 AH_TEMPLATE([BUILDDIR],[tor's build directory])
 AC_DEFINE_UNQUOTED(BUILDDIR,"$BUILDDIR")
 
+if test "x$SRCDIR" = "x"; then
+  SRCDIR=${srcdir}
+fi
+AC_SUBST(SRCDIR)
+AH_TEMPLATE([SRCDIR],[tor's source directory])
+AC_DEFINE_UNQUOTED(SRCDIR,"$SRCDIR")
+
 if test "x$CONFDIR" = "x"; then
   CONFDIR=`eval echo $sysconfdir/tor`
 fi
diff --git a/src/test/test_entrynodes.c b/src/test/test_entrynodes.c
index 1be0ce1..3c73a0b 100644
--- a/src/test/test_entrynodes.c
+++ b/src/test/test_entrynodes.c
@@ -115,7 +115,7 @@ static void *
 fake_network_setup(const struct testcase_t *testcase)
 {
   /* This is the file containing our test descriptors. */
-  const char *fname = BUILDDIR "/src/test/test_descriptors.txt";
+  const char *fname = SRCDIR "/src/test/test_descriptors.txt";
 
   (void) testcase;
 
Last edited 5 years ago by weasel (previous) (diff)

comment:2 Changed 5 years ago by weasel

That's actually pretty ugly, though:

Depending on how you call configure, you end up with different contents of the define:

> weasel@valiant:~/projects/tor/tor/build-tree$ ~/projects/tor/tor/configure  && grep DIR *h
> #define SRCDIR "/home/weasel/projects/tor/tor"
> weasel@valiant:~/projects/tor/tor/build-tree$ ../configure  && grep DIR *h
> #define SRCDIR ".."

Changed 5 years ago by weasel

I like this version slightly better.

comment:3 Changed 5 years ago by weasel

Status: newneeds_review

comment:4 Changed 5 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Whoops. I checked in another fix for this before I saw your ticket. (The final version was 7259e3f6049c7a644cb1c797432d29b57a.) Your fix is pretty clean too. I'm going to close this for now, but if we need to add more data files for use by the unit tests, let's do it your way.

One thing that we would need to fix to solve this your way: I don't think it will work with Windows and Mingw. (I tried a similar approach and Windows unit tests failed.) Here's what I think was happening: Under mingw, doing a pwd will get you a directory with a nice name like /c/home/weasel/src/tor. But when compiled program tries to open /c/home/weasel/{...}/test/test_descriptors.txt , it will fail, because the Windows open() implementation expects filenames like C:\home\weasel\{...}\test_descriptors.txt . (I think it can handle the forward-slashes, but not the /c/.)

comment:5 Changed 5 years ago by nickm

Another way to solve this kind of thing in the future would be to force automake to copy the data file into $builddir/src/test if it isn't already there. That could be a bit ugly though.

Note: See TracTickets for help on using tickets.