Skip to content

Conversation

@damienmarchal
Copy link
Contributor

The initial problem is that the SP3_add_python_package (and few others) was using
configure_file() to install python package into the build dir (as well as install dir).

The problem is that configure_file, because of the variable expansion, create a link
between the file and CMake...so each time the file is touched, cmake is re-executed.
To me it make no sense to use configure for every .py unless you really want variable expansion (and thus have an
explicit filename.py.in) for that.

So I refactor the whole thing to:

  • stop using configure_file
  • make a link between the python files from the source dir to the build dir, so
    (so we can now edit python bug without noticing too late that we touched the files in build directory).
  • make an install process that copy the files (and not the links)
  • put all the test-files at the same location in various tests

The initial problem is that the SP3_add_python_package (and few others) was using
configure_file() to install python package into the build dir (as well as install dir).

The problem is that configure_file, because of the variable expansion, create a link
between the file and CMake...so each time the file is touched, cmake is re-executed.
To me it make no sense to use configure for every .py unless you really want variable expansion (and thus have an
explicit filename.py.in) for that.

So I refactor the whole thing to:
- stop using configure_file
- make a link between the python files from the source dir to the build dir, so
  (so we can now edit python bug without noticing too late that we touched the files in build directory).
- make an install process that copy the files (and not the links)
- put all the test-files at the same location in various tests
@jnbrunet jnbrunet self-assigned this Nov 9, 2020
@jnbrunet jnbrunet added this to the V20.12 milestone Nov 9, 2020
@jnbrunet
Copy link
Contributor

jnbrunet commented Nov 9, 2020

Hey @damienmarchal

Just a small note: your PR isn't forgotten. We are working on putting back the CI on its foot. Then I'll rebase your branch and merge it if the tests passes.

@jnbrunet
Copy link
Contributor

jnbrunet commented Dec 4, 2020

Hey @damienmarchal ,

I've updated your branch with master, however I do not have the permission to push to it. Do you want me to create a new PR with the updated branch?

@jnbrunet jnbrunet modified the milestones: V20.12, V21.06 Dec 23, 2020
@hugtalbot
Copy link
Contributor

I see this is not active anymore since September, should we close it @damienmarchal ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

issue: bug Something isn't working pr: status to review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants