Ticket #5767 (closed: fixed)

Opened 8 years ago

Last modified 5 years ago

CMake: Ensure include paths have only what is necessary for each target

Reported by: Russell Taylor Owned by: Russell Taylor
Priority: minor Milestone: Release 2.3
Component: Mantid Keywords:
Cc: martyn.gigg@… Blocked By:
Blocking: Tester: Alex Buts

Description (last modified by Russell Taylor) (diff)

Because of the fact that an include_directories command in CMake has file scope (so targets will pick such a command up even if it comes later in the CMakeLists), many of our libraries have extra directories in the path that are required for the unit tests. The solution is to add CMakeLists files to all out test directories and move the creation of the test executable targets to there.

We also have things (e.g. Qt) that are added to the include path for everywhere when they should be added only for those targets that need them.

Doing this will mean that trying to use something that isn't a dependency will be picked up sooner.

Change History

comment:1 Changed 8 years ago by Russell Taylor

  • Status changed from new to accepted

comment:2 Changed 8 years ago by Russell Taylor

  • Description modified (diff)

comment:3 Changed 8 years ago by Russell Taylor

Re #5767. Move creation of unit test targets into new CMakeLists

...in test directory. This means that any include paths needed for the unit tests are not also added to the main library build command.

The unit test files are still declared in the same place so that people only need to edit one file when adding a class.

Changeset: 86d69dfb469029482d1d5cf3d4e839dc13da5817

comment:4 Changed 8 years ago by Russell Taylor

Re #5767. Only include Qt, Python & testing frameworks where needed.

Changeset: f43a7d34222b2996f9e6d6fe5fdd36fa9899f528

comment:5 Changed 8 years ago by Russell Taylor

Re #5767. Give variable same name on Windows as elsewhere.

Changeset: 8faa2156653ecde27216cc2f6b66cd6e660e10e5

comment:6 Changed 8 years ago by Russell Taylor

Re #5767. Add python include path for MantidQt/Plot.

Changeset: 78e89ee7278738e610f271d545d687cb29b0f1d6

comment:7 Changed 8 years ago by Russell Taylor

Re #5767. Remove erroneous line.

Changeset: 97fac77e34b135920fb2edbde237f25ec42df507

comment:8 Changed 8 years ago by Russell Taylor

Re #5767. Suppress warnings out of Qt code.

Changeset: 5750d6e4d4a49c9d739e12d334e86740087facb1

comment:9 Changed 8 years ago by Russell Taylor

Re #5767. More CMake re-jigging to get include paths correct/minimal.

Changeset: 65c4ea835e1efed384d071fd588d391e1afff3cc

comment:10 Changed 8 years ago by Russell Taylor

Re #5767. Precise fix?

Changeset: 5307afb3ca92c5badc403b5aee5527dd71c3f924

comment:11 Changed 8 years ago by Michael Reuter

Refs #5767. Fix to deal with precise issue.

Changeset: 99a51c67c1f2e7c2b2ccf505c009c2850034ca77

comment:12 Changed 8 years ago by Russell Taylor

  • Status changed from accepted to verify
  • Resolution set to fixed

This should be all sorted now. The primary test is that everything still builds successfully. To dig deeper, check the 'Additional includes' project properties in a Visual Studio build or inspect the -I/-isystem arguments when doing a 'make VERBOSE=1'. There should be no include paths that are not necessary for a particular package to build.

comment:13 Changed 8 years ago by Russell Taylor

Re #5767. Move creation of unit test targets into new CMakeLists

...in test directory. This means that any include paths needed for the unit tests are not also added to the main library build command.

The unit test files are still declared in the same place so that people only need to edit one file when adding a class.

Changeset: 86d69dfb469029482d1d5cf3d4e839dc13da5817

comment:14 Changed 8 years ago by Russell Taylor

Re #5767. Only include Qt, Python & testing frameworks where needed.

Changeset: f43a7d34222b2996f9e6d6fe5fdd36fa9899f528

comment:15 Changed 8 years ago by Russell Taylor

Re #5767. Give variable same name on Windows as elsewhere.

Changeset: 8faa2156653ecde27216cc2f6b66cd6e660e10e5

comment:16 Changed 8 years ago by Russell Taylor

Re #5767. Add python include path for MantidQt/Plot.

Changeset: 78e89ee7278738e610f271d545d687cb29b0f1d6

comment:17 Changed 8 years ago by Russell Taylor

Re #5767. Remove erroneous line.

Changeset: 97fac77e34b135920fb2edbde237f25ec42df507

comment:18 Changed 8 years ago by Russell Taylor

Re #5767. Suppress warnings out of Qt code.

Changeset: 5750d6e4d4a49c9d739e12d334e86740087facb1

comment:19 Changed 8 years ago by Russell Taylor

Re #5767. More CMake re-jigging to get include paths correct/minimal.

Changeset: 65c4ea835e1efed384d071fd588d391e1afff3cc

comment:20 Changed 8 years ago by Russell Taylor

Re #5767. Precise fix?

Changeset: 5307afb3ca92c5badc403b5aee5527dd71c3f924

comment:21 Changed 8 years ago by Michael Reuter

Refs #5767. Fix to deal with precise issue.

Changeset: 99a51c67c1f2e7c2b2ccf505c009c2850034ca77

comment:22 Changed 8 years ago by Alex Buts

  • Status changed from verify to verifying
  • Tester set to Alex Buts

comment:23 Changed 8 years ago by Alex Buts

  • Status changed from verifying to closed

Does what is says.

The dependencies in Win projects greatly simplified. File lists do not need unnecessary folder prefix to work.

comment:24 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue 6613

Note: See TracTickets for help on using tickets.