Ticket #5767 (closed: fixed)
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: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