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

file COPY cannot set permissions for 05.catkin_make.bash #718

Closed
k-okada opened this issue Feb 13, 2015 · 13 comments
Closed

file COPY cannot set permissions for 05.catkin_make.bash #718

k-okada opened this issue Feb 13, 2015 · 13 comments

Comments

@k-okada
Copy link
Contributor

k-okada commented Feb 13, 2015

we sometimes see following error when we compile multiple packages in parallel, I still can not find a way to reproduce this issue perfectly, but it occurs sometimes. It seems we have to not to overwrite on
https://github.com/ros/catkin/blob/indigo-devel/cmake/catkin_add_env_hooks.cmake#L91, or find any other way to write atomic copy.

[jsk_recognition_msgs] CMake Error at /opt/ros/indigo/share/catkin/cmake/catkin_add_env_hooks.cmake:91 (file): 
[jsk_recognition_msgs]   file COPY cannot set permissions on 
[jsk_recognition_msgs]   "/workspace/ros/ws_jsk_recognition/devel/etc/catkin/profile.d/05.catkin_make.bash" 
[jsk_recognition_msgs] Call Stack (most recent call first): 
[jsk_recognition_msgs]   /opt/ros/indigo/share/catkin/cmake/all.cmake:198 (catkin_add_env_hooks) 
[jsk_recognition_msgs]   /opt/ros/indigo/share/catkin/cmake/catkinConfig.cmake:20 (include) 
[jsk_recognition_msgs]   CMakeLists.txt:3 (find_package) 
[jsk_recognition_msgs]  
@dirk-thomas
Copy link
Member

Please provide an example command sequence for which it at least happens sometimes.

@k-okada
Copy link
Contributor Author

k-okada commented Feb 16, 2015

ok, I finally able to find the way to reproduce this issue, see https://travis-ci.org/k-okada/test_catkin_parallel/jobs/50891940#L2704

@wjwwood
Copy link
Member

wjwwood commented Feb 16, 2015

So the file in question (based on your travis-ci job) being installed which presumably collides is 05.catkin_make_isolated.bash, but my question is why is this file getting installed for every package? It is the tab completion for catkin_make_isolated, that should only get installed by catkin right?

If you build multiple packages in parallel which create the same file then you can get a collision like this. The question is why is this file being installed by more than one package.

In the original post it is the tab completion for catkin_make which collides. The installation of these files seems to be done in all.cmake in catkin:

catkin/cmake/all.cmake

Lines 197 to 203 in 2599eb9

if(CMAKE_HOST_UNIX)
catkin_add_env_hooks(05.catkin_make SHELLS bash DIRECTORY ${catkin_EXTRAS_DIR}/env-hooks ${catkin_skip_install_env_hooks})
catkin_add_env_hooks(05.catkin_make_isolated SHELLS bash DIRECTORY ${catkin_EXTRAS_DIR}/env-hooks ${catkin_skip_install_env_hooks})
catkin_add_env_hooks(05.catkin-test-results SHELLS sh DIRECTORY ${catkin_EXTRAS_DIR}/env-hooks ${catkin_skip_install_env_hooks})
else()
catkin_add_env_hooks(05.catkin-test-results SHELLS bat DIRECTORY ${catkin_EXTRAS_DIR}/env-hooks ${catkin_skip_install_env_hooks})
endif()

@dirk-thomas why is that being done?

@k-okada
Copy link
Contributor Author

k-okada commented Feb 17, 2015

thanks for looking into, I found that we sometimes have copy error on both 05.catkin_make.bash or 05.catkin_make_isolated.bash, and my testcode uses https://github.com/k-okada/test_catkin_parallel/blob/master/test1/CMakeLists.txt, which I believe very normal.

it seems catkin checks if the 05.catkin_make_isolated.bash exists (

elseif (EXISTS ${base})
), but it seems not working for timing issue. Another question is Is install command atomic or not.

@dirk-thomas
Copy link
Member

@wjwwood Indeed, the completion scripts could be registered only once for catkin itself.

@k-okada I can't tell why the operation fails. It could be any kind of system reason: permission denied, no space left on device, destination directory does not exist, etc. CMake does not report the actual cause (http://public.kitware.com/Bug/view.php?id=14700). The travis log you referenced does not involve an install. The files in the devel space are created using a file(COPY ...):

file(COPY ${base} DESTINATION ${CATKIN_DEVEL_PREFIX}/etc/catkin/profile.d)

@k-okada
Copy link
Contributor Author

k-okada commented Feb 17, 2015

it only happens if you build multiple catkin packag and it happens randomly, so may be file(COPY...) is not atomic function. It seems you have implemented
https://github.com/ros/catkin/blob/2599eb973762c2a23f103fdc8bd0ef40e779f2ed/cmake/atomic_configure_file.cmake, and I'd like to know the reason. did you find that configure_file is not atomic but file(RENAME is atimic?

@dirk-thomas
Copy link
Member

configure_file is not atomic (it is being created and populated incrementally) but as far as I remember the issue is that it was being used in the middle of that already.

For the problem you are describing that doesn't seem to be a problem but it seems to be unable to perform the copy.

@wjwwood
Copy link
Member

wjwwood commented Feb 17, 2015

I believe that copy is never atomic, only rename. But rename either doesn't work across filesystems or is no longer atomic when used across filesystems, I don't remember which. I do remember talking about creating a peer file in the destination folder with a random name and then using rename to rename the randomly named file to the target name. This is both atomic and guaranteed to work since the file being renamed is always on the same filesystems since they are in the same folder.

But that's not the point, I think the important question here is _why_ are these particular files getting installed by more than one package? I would have thought only catkin would install these files. We've taken pains to make sure that files written by multiple packages due to catkin are done using the rename action as their final step, but if configure file or copy are used to create other files which are known to be written to the same place then this error will continue to happen.

Another question is Is install command atomic or not.

It's not considered to be atomic, but as @dirk-thomas said, this isn't an install problem, this collision is happening in the develspace.

@dirk-thomas
Copy link
Member

Some of the environment hooks (the ones for completion) do not need to be installed for other packages than catkin. That is easy to fix.

But some of the environment hooks (setting the variables for test results) must be installed for every package. So for them the copy to the devel space needs to be atomic if their destination across packages is the same and they are being invoked in concurrently.

@simonlynen
Copy link

@dirk-thomas @wjwwood +1 on this issue. Do you have a workaround for this?

@wjwwood
Copy link
Member

wjwwood commented Mar 9, 2015

We do not. We need to sit down and discuss it in more detail. This is a conceptual problem and we'll likely have to adjust how we do testing (or at least how we pass the test directory to rostest) to make it work.

@wjwwood
Copy link
Member

wjwwood commented Mar 9, 2015

This is related to #719

@dirk-thomas dirk-thomas changed the title fild COPY cannot set permissions for 05.catkin_make.bash file COPY cannot set permissions for 05.catkin_make.bash Apr 13, 2015
@dirk-thomas
Copy link
Member

Some of the environment hooks have been removed in b298bb1#diff-4711d7d40543d1cda255cb4f83806100

The remaining ones should only be added for the catkin package: #732

Therefore I will close this ticket since it should not be an issue anymore.

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

4 participants