search

tum-i4 / input-dependency-analyzer

7 stars 4 forks source link

Modernize CMakeLists.txt #116

Open dennisfischer opened 6 years ago

dennisfischer commented 6 years ago

The style of the used CMakeLists.txt is not using any of the best practices and recommendation of modern CMake. This creates several problems if the input-dependency-analyzer is used in other projects.

Problem and Motivation

  1. It is not possible to include the project with something like find_package() because the CMakeLists.txt does not create the necessary files. Instead, only the header files are installed.
  2. A related problem to 1) exists. It is not possible to use input-dependency-analyzer project in a "super-project". This means that I cannot create a new CMakeLists.txt which builds input-dependency-analyzer and a secondary project like oblivious-hashing in a single step. Instead, input-dependency-analyzer needs to be installed first.
  3. The current install of headers contains a minor bug. Headers are copied to /usr/local/include/input-dependency. Analysis and Transforms both make use of a Utils.h but only the Utils.h from one of both subdirectories can be exported due to the same filename.
  4. The build dependencies and requirements to build CMake targets are hidden behind calls like include_directories(), add_definitions(..), etc. These can be expressed better with target_ functions.
  5. Projects like self-checksumming often use relative paths to the input-dependency-analyzer project.

Solution

I propose to update the CMakeLists.txt files used in this project. I will provide a Pull request with updated CMakeLists.txt files that solves the above problems. However, in its current state two problems exist within the source files which need to be discussed first.

  1. How do we resolve the conflicting Utils.h from Analysis and Transforms?
  2. All source files currently use relative paths, e.g., #include "BasicBlockAnalysisResult.h". In this update we should replace this with #include "input-dependency/BasicBlockAnalysisResult.h" at the very least. To solve 1) we could move one or both of the subprojects to their own folders. An include could then look like this: #include "input-dependency/Analysis/BasicBlockAnalysisResult.h"

Let me know what you think.

dennisfischer commented 6 years ago

@anahitH In your working branch dep-graph-based-input-dep you added an include and lib folder for PDG. Is PDG in this sense part of the input-depedency library or is it a "different" project.

I'm asking such that I can prepare another PR to update the CMakeLists.txt for this branch. I'd assume it is part of this project. This creates a minor problem with directory names then. If the PDG include and lib folder are both prefixed with input-dependency, then the names of Analysis and analysis clash. I'd therefore move the analysis folder of PDG to the PDG folder as a subdirectory.

anahitH commented 6 years ago

@dennisfischer thanks for updating CMakefiles. PDG is going to be a part of input dependency library. In fact I'm going to rewrite input dependency analysis using PDG. dep-graph-based-inpu-dep is not fully functioning right now, so I would suggest that we leave it for now.

dennisfischer commented 6 years ago

@anahitH Then we should move the analysis subdirectory in PDG one level deeper. Then I could provide a better CMakeLists.txt for this branch too.