ament / ament_cmake

Supporting CMake packages for working with ament
Apache License 2.0
98 stars 123 forks source link

No executable found for scripts when using --symlink-install and install(PROGRAMS) #545

Open yschulz opened 3 weeks ago

yschulz commented 3 weeks ago

Redirected from colcon-core #638

TLDR On Linux systems: Exectuables installed without --symlink-install gain executing permissions fine. Exectuables installed with --symlink-install do not gain executing permission because symbolic links do not have their own permissions.

@cottsay suggested one two things: _1. Detect this scenario and fall back to the non-symlink behavior while possibly displaying a warning

  1. Refuse to do anything and demand that the developer make the original script executable so that we can proceed with the symlink_

I would strongly vote for solution 1. as the loss of permissions may easily be coming from an automation pipeline or such. Refusal would be terrible in a scenario like that.

clalancette commented 3 weeks ago

I would strongly vote for solution 1. as the loss of permissions may easily be coming from an automation pipeline or such. Refusal would be terrible in a scenario like that.

The thing about doing 1 is that it completely breaks the expectation of --symlink-install. While printing a warning in that scenario is nice, the fact of the matter is that people don't read warnings, and so will become frustrated when --symlink-install doesn't work how they would expect. Thus, my suggestion would be for 2, though there are some complications there in determining what "should" be an executable or not.

yschulz commented 3 weeks ago

I see your point, but personally I still rather have things work with a warning than break at a point that sometimes still work under different circumstances. Then again, I rather build verbosely and someone else may not.

Anyhow, being more specific about the situation using the PROGRAMS flag, which specifies the developers intent clearly, the distinction for 2 would be implicit (as the non-sym version does give the permission if not yet set).

cottsay commented 3 weeks ago

Worth noting is that git can store executable permissions on files just fine. If ament were to process the non-executable script as an error (option 2), the developer could simply change the script to be executable, right?

yschulz commented 3 weeks ago

Git does, yes. But let say before you ship your code, you build it first somewhere else to test. Then you rather copy, compress or do something else, rather than cloning it somewhere else. Might work 99% of cases, but I would hate that 1%.

Just a thought, I discovered this bug by quickly creating a script with a umask that didnt set it to executable :P Number 2 is totally fine