Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

REGEX REPLACE in CMakeLists.txt fails for complex directory paths #2069

Closed
Pathorse opened this issue Oct 17, 2023 · 6 comments
Closed

REGEX REPLACE in CMakeLists.txt fails for complex directory paths #2069

Pathorse opened this issue Oct 17, 2023 · 6 comments
Assignees

Comments

@Pathorse
Copy link

Pathorse commented Oct 17, 2023

Bug description

Pinocchio build fails when using complex (symbolic) directory paths, e.g., /test_pinnochio_build/1.0+gitAUTOINC+8ajb23j4b/pinocchio/build, as REGEX REPLACE fails to correctly amend strings before attempting to create symbolic links (or copying).

This issue was found while using pinocchio with the Yocto SDK.

Expected behavior

The build should not fail for any directory paths given.

Reproduction steps

Steps to reproduce the behavior:

  1. Create a test folder:
mkdir -p test_pinnochio_build/1.0+gitAUTOINC+8ajb23j4b
  1. Clone Pinocchio
git clone git@github.com:stack-of-tasks/pinocchio.git
  1. Initialize submodules
cd  test_pinnochio_build/1.0+gitAUTOINC+8ajb23j4b/pinocchio
git submodule update --init
  1. Build
mkdir build && cd build
cmake ..
  1. See error

Solution

The problem can be solved by replacing REGEX REPLACE with REPLACE, which given the fact that the desired string operation doesnt require the flexibility of regular expressions may make sense to move away from anyways (unless I am missing something here which might be the case as I am pretty new with Pinocchio). The swap must be done in three files:

1. Toplevel CMakeLists.txt
2. Python bindings CMakeLists.txt
3. Unittest CMakelists.txt

System

  • OS: Ubuntu 22.04
  • Pinocchio version: 2.6.20
@jcarpent
Copy link
Contributor

Thanks @Pathorse for the nice report. @nim65s @jorisv Could you have a look to this issue in more depth?

@nim65s
Copy link
Contributor

nim65s commented Oct 17, 2023

I think we should simply switch to a more standard CMake use, and:

  1. stop simlinking stuff around
  2. list all header / sources in CMakeLists.txt

If agreed, I probably can work on it this week, but probably it would be better to do this on the private branch for v3

@jorisv
Copy link
Contributor

jorisv commented Oct 25, 2023

@Pathorse This issue should be fixed by #2070

@Pathorse
Copy link
Author

Thanks!

@manumerous
Copy link

Thanks a lot!

@jcarpent
Copy link
Contributor

Solve via #2070

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants