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

Fix a race condition during CPack packaging when creating symlink on Apple #732

Merged
merged 1 commit into from
Sep 2, 2024

Conversation

jmarrec
Copy link
Collaborator

@jmarrec jmarrec commented Sep 2, 2024

cf #727 (comment)

I can reproduce the error even locally on Ubuntu:

option(USE_TWO_EXECUTE "Use two execute_process instead of a single one" OFF)

if(USE_TWO_EXECUTE)
  execute_process(
    COMMAND echo "CMAKE_INSTALL_PREFIX = ${CMAKE_INSTALL_PREFIX}"

    COMMAND "${CMAKE_COMMAND}" -E make_directory "bin"

    COMMAND_ECHO STDOUT
    COMMAND_ERROR_IS_FATAL ANY
  )

  execute_process(
    COMMAND "${CMAKE_COMMAND}" -E create_symlink
      "../OpenStudioApp.app/Contents/MacOS/openstudio"
      "bin/openstudio"

    COMMAND_ECHO STDOUT
    COMMAND_ERROR_IS_FATAL ANY
  )
else()
  execute_process(
    COMMAND echo "CMAKE_INSTALL_PREFIX = ${CMAKE_INSTALL_PREFIX}"

    COMMAND "${CMAKE_COMMAND}" -E make_directory "bin"

    COMMAND "${CMAKE_COMMAND}" -E create_symlink
      "../OpenStudioApp.app/Contents/MacOS/openstudio"
      "bin/openstudio"

    COMMAND_ECHO STDOUT
    COMMAND_ERROR_IS_FATAL ANY
  )
endif()
for i in {0..20}; do rm -Rf bin && cmake -DUSE_TWO_EXECUTE:BOOL=OFF . || (echo "FAILED" && break); done

image

for i in {0..20}; do rm -Rf bin && cmake -DUSE_TWO_EXECUTE:BOOL=ON . || (echo "FAILED" && break); done

```cmake
o execute_process instead of a single one" OFF)

if(USE_TWO_EXECUTE)
  execute_process(
    COMMAND echo "CMAKE_INSTALL_PREFIX = ${CMAKE_INSTALL_PREFIX}"

    COMMAND "${CMAKE_COMMAND}" -E make_directory "bin"

    COMMAND_ECHO STDOUT
    COMMAND_ERROR_IS_FATAL ANY
  )

  execute_process(
    COMMAND "${CMAKE_COMMAND}" -E create_symlink
      "../OpenStudioApp.app/Contents/MacOS/openstudio"
      "bin/openstudio"

    COMMAND_ECHO STDOUT
    COMMAND_ERROR_IS_FATAL ANY
  )
else()
  execute_process(
    COMMAND echo "CMAKE_INSTALL_PREFIX = ${CMAKE_INSTALL_PREFIX}"

    COMMAND "${CMAKE_COMMAND}" -E make_directory "bin"

    COMMAND "${CMAKE_COMMAND}" -E create_symlink
      "../OpenStudioApp.app/Contents/MacOS/openstudio"
      "bin/openstudio"

    COMMAND_ECHO STDOUT
    COMMAND_ERROR_IS_FATAL ANY
  )
endif()
```

```shell
for i in {0..20}; do rm -Rf bin && cmake -DUSE_TWO_EXECUTE:BOOL=OFF . || (echo "FAILED" && break); done

for i in {0..20}; do rm -Rf bin && cmake -DUSE_TWO_EXECUTE:BOOL=ON . || (echo "FAILED" && break); done
```
Comment on lines +475 to +481

COMMAND_ECHO STDOUT
COMMAND_ERROR_IS_FATAL ANY
)

# We break it into two `execute_process`es so that there is no race condition between the make_directory above and the create_symlink below
execute_process(
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix

@macumber
Copy link
Collaborator

macumber commented Sep 2, 2024

Nice!

@jmarrec jmarrec merged commit 2d13a50 into develop Sep 2, 2024
10 checks passed
@jmarrec jmarrec deleted the fix_cpack branch September 2, 2024 15:41
@github-actions github-actions bot locked and limited conversation to collaborators Sep 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants