Copied type_description_interfaces structs (rep2011)#732
Conversation
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
clalancette
left a comment
There was a problem hiding this comment.
Besides the changes inline, I'm wondering if we should add a git hook or github action to check if someone manually changes these files. I'd prefer the git hook (that way it will inform them locally when they are committing), but if we can't make that happen then a github action to check this would be nice.
…e copy is necessary Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
|
@clalancette I'm thinking about how a hook could check on this. At the end of the day we don't know where the built code being copied actually came from so it can't be fully proofed against fools or ne'erdowells. Maybe some "fingerprint" separate file that's also checked in - and that file can be just a big mean warning and one piece of information like the git commit of And then the git hook could simply verify that this file is also changed alongside changes to any of the copied sources? |
|
Ah - I forgot. git hooks aren't checked in, you can't enforce their use by end users except by extra setup steps asking them to set up hooks for their local clone. The only way to make this policy enforceable is definitely a github action |
c024954 to
b255a85
Compare
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
b255a85 to
0071de8
Compare
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
|
OK - I've checked in a github action that looks for changed files, and fails immediately if "fingerprint" is changed without sources, or vice versa. If both are changed, then it looks for the latest commit on Cases testing:
|
6d7888c to
24cad69
Compare
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
24cad69 to
c7ff779
Compare
|
OK, second pass. I've changed the fingerprint file to be an output of hashes of the files that got copied. The builtin check here only checks that the fingerprint file changes when the copied sources do, but the big mean warning should be what warns folks off from changing it manually. Cases testing:
|
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
968eb20 to
6dbfd35
Compare
|
Gist: https://gist.githubusercontent.com/emersonknapp/583f883f05c5405351ac612c1c628d1f/raw/5d9dbe9f73ad23684d544bf0ace2955160cf2340/ros2.repos arm failed in infrastructure and auto-triggered rebuild |
Part of ros2/ros2#1159
Reviewers please see scripts/copy_type_description_generated_sources.bash which creates these copies - the manual changes in this review are
rosidl_runtime_c/CMakeLists.txtrosidl_runtime_c/docs/FEATURES.mdrosidl_runtime_cpp/CMakeLists.txtrosidl_runtime_cpp/docs/FEATURES.mdscripts/copy_type_description_generated_sources.bash