borglab / gtsam

GTSAM is a library of C++ classes that implement smoothing and mapping (SAM) in robotics and vision, using factor graphs and Bayes networks as the underlying computing paradigm rather than sparse matrices.
http://gtsam.org
Other
2.55k stars 754 forks source link

GTSAM_SINGLE_TEST_EXE option causes compile errors #91

Closed cntaylor closed 2 years ago

cntaylor commented 5 years ago

Description

If the GTSAM_SINGLE_TEST_EXE option is enabled, then the unit tests fail to compile. For example, check.geometry will not compile under either Linux or Windows. It generates a "multiple definition" error

Steps to reproduce

  1. Turn on the GTSAM_SINGLE_TEST_EXE flag in cmake
  2. Run make check.geometry

Expected behavior

Expect the unit tests to compile without errors.

Environment

This occurs in both Windows and Linux. That said, the GTSAM_SINGLE_TEST_EXE is turned on by default in Windows, but is off by default in Linux. So, this is far more "noticeable" in Windows.

Additional information

Two questions really.

  1. Is there a reason this is turned on by default in Windows? Or a reason for this flag at all?
  2. There are a lot of duplicate tests as you go across the individual programs in "check.geometry" (and lots of other groups of "check" programs). Is there a reason for these duplicates? We can go through and make each test individually names, or we could remove some of the duplicates.
ProfFan commented 4 years ago

121

ProfFan commented 4 years ago

@jlblancoc Do we still have this issue? I think you added AppVeyor CI for Win32 platforms...

jlblancoc commented 4 years ago

Yes, but it failed for a weird reason that couldn't replicate locally in MSVC. The error message is:

-- Configuring done
-- Generating done
-- Build files have been written to: C:/projects/gtsam/build
The solution file has two projects named "gtsam_examples".

and I got stuck there so...

Apparently, it has nothing to do with the issue in this thread.

ProfFan commented 4 years ago

@jlblancoc I think the problem is with MSBuild. Could you test locally with MSBuild instead of VS?

jlblancoc commented 4 years ago

@ProfFan : thanks for the pointer. Using msbuild it fails, but invoking cmake --build . it works (however it internally invokes cl!)

ProfFan commented 4 years ago

microsoft/msbuild#3019

ProfFan commented 4 years ago

Is this still an issue?

varunagrawal commented 3 years ago

This is still an issue. There is name-clashing of global variables and functions happening when we merge various test files.

varunagrawal commented 3 years ago

So after some consideration, this issue can be easily solved if we update the namespaces of each of the test files. This is an easy thing to do, but requires a lot of legwork to check and run the tests.