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

Use libexec to install lib binaries #279

Merged
merged 2 commits into from
May 5, 2022
Merged

Conversation

j-rivero
Copy link
Contributor

We are installing the cli11 binaries in under the standard library path which is something that does not fit well according to the FHR. libexec seems a better place for them:

4.7.1. Purpose

/usr/libexec includes internal binaries that are not intended to be executed directly by users or shell scripts. Applications may use a single subdirectory under /usr/libexec.

Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>
@github-actions github-actions bot added 🌱 garden Ignition Garden 🏯 fortress Ignition Fortress labels Nov 25, 2021
@codecov
Copy link

codecov bot commented Nov 25, 2021

Codecov Report

Merging #279 (9c3e190) into ign-transport11 (b470925) will decrease coverage by 22.40%.
The diff coverage is n/a.

❗ Current head 9c3e190 differs from pull request most recent head c1488d6. Consider uploading reports for the commit c1488d6 to get more accurate results

@@                 Coverage Diff                  @@
##           ign-transport11     #279       +/-   ##
====================================================
- Coverage            89.07%   66.66%   -22.41%     
====================================================
  Files                   51        1       -50     
  Lines                 4777       24     -4753     
====================================================
- Hits                  4255       16     -4239     
+ Misses                 522        8      -514     
Impacted Files Coverage Δ
src/NetUtils.cc
log/src/MsgIter.cc
log/src/Batch.cc
src/Uuid.cc
log/src/QueryOptions.cc
src/TopicStatistics.cc
src/Discovery.cc
src/NodeOptionsPrivate.hh
include/ignition/transport/TopicStatistics.hh
...og/include/ignition/transport/log/QualifiedTime.hh
... and 42 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b470925...c1488d6. Read the comment docs.

@chapulina
Copy link
Contributor

Do we need any changes to the -release repo?

@j-rivero
Copy link
Contributor Author

Do we need any changes to the -release repo?

Yes! gazebo-release/gz-transport11-release#4

@mjcarroll
Copy link
Contributor

The CLI binaries can still be executed standalone by a user if they don't wish to use ign, so I don't know if libexec is the correct location for them.

@chapulina
Copy link
Contributor

@mjcarroll has a good point. Maybe we need to create new ignition-transport11 packages for the binaries?

@chapulina chapulina removed the 🌱 garden Ignition Garden label Dec 30, 2021
@chapulina
Copy link
Contributor

Maybe we need to create new ignition-transport11 packages for the binaries?

I believe this is what's being suggested for plugin2 here, right @j-rivero ?

@j-rivero
Copy link
Contributor Author

j-rivero commented May 2, 2022

Yep, the same.

@j-rivero
Copy link
Contributor Author

j-rivero commented May 5, 2022

Updated the -release repo PR gazebo-release/gz-transport11-release#4

@j-rivero j-rivero merged commit 862f8e0 into ign-transport11 May 5, 2022
@j-rivero j-rivero deleted the jrivero/use_libexec branch May 5, 2022 17:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏯 fortress Ignition Fortress
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants