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

Make IKOS available in the spaceros image (#99). #123

Merged
merged 1 commit into from
Jan 26, 2024
Merged

Conversation

ivanperez-keera
Copy link
Contributor

IKOS is one of the tools that Space ROS includes to perform static analysis of ROS 2 applications. Although ament_ikos is provided with the docker image, IKOS itself is not, rendering the former useless and leaving it to users to compile and install IKOS prior to being able to use it.

This commit modifies the spaceros Earthfile to install IKOS globally in the system.

@ivanperez-keera
Copy link
Contributor Author

ivanperez-keera commented Jan 25, 2024

Filing this as a draft. I built it locally and checked that ikos was building and running during compilation. I'd like to check that it's available in the result. I could use a couple of extra eyes. It does not take a hugely long time to compile.

EDIT: I just checked that it's available in the result. I had to move the IKOS build to the build target so that the installed binaries remain in the final image. The directory /opt/ikos/bin has to be added to the PATH, which this change does not do, but that's a relatively minor issue IMO. I don't mind that it is that way right now (unless it's trivial to fix) and we just tell users to adjust the PATH on their end.

@ivanperez-keera ivanperez-keera marked this pull request as ready for review January 25, 2024 15:37
@ivanperez-keera ivanperez-keera added this to the humble-2024.01.0 milestone Jan 25, 2024
IKOS is one of the tools that Space ROS includes to perform static analysis of
ROS 2 applications. Although ament_ikos is provided with the docker image, IKOS
itself is not, rendering the former useless and leaving it to users to compile
and install IKOS prior to being able to use it.

This commit modifies the spaceros Earthfile to install IKOS globally in the
system.
@ivanperez-keera
Copy link
Contributor Author

This is ready for review, if anyone wants to take it.

@xfiderek
Copy link
Contributor

Have you checked if #104 is fixed by this PR? If not i can dig into artifact output and check.

@ivanperez-keera
Copy link
Contributor Author

@xfiderek I have not, and I don't want to add that to this release. We are getting dangerously close; I think it's best that we first close the items assigned to this release that we have not finished yet.

@ivanperez-keera
Copy link
Contributor Author

ivanperez-keera commented Jan 26, 2024

@xfiderek can I trouble you to review this PR? It can't be merged until someone approves. Thanks!

@ivanperez-keera
Copy link
Contributor Author

Upps. "At least 1 approving review is required by reviewers with write access"

@ivanperez-keera
Copy link
Contributor Author

Sorry, @xfiderek . It appears that it needs a review also from someone with write access.

I've asked for an additional review.

Thanks for approving it though :)

@xfiderek
Copy link
Contributor

xfiderek commented Jan 26, 2024

Ok, btw after this PR is merged lets create an issue to track status of apt availability, something like "install ikos from deb". Does it make sense?

@ivanperez-keera
Copy link
Contributor Author

Maybe we can have a quick talk about how to best do this. I'm available to talk / call.

We definitely need to track it, but what's odd about this kind of issue is that instead of us doing something, it's in "waiting" or "on hold" until someone else does something. Perhaps it should be a discussion instead of an issue, or it should be a task in my personal agenda as opposed to an issue in the docker.

There's also the possibility that they might say "no" to both accepting IKOS in debian as non-free, and changing the license to accept it in main. In that case, the issue might not be "install ikos from deb", but something else.

@ivanperez-keera
Copy link
Contributor Author

And just for full context: https://ftp-master.debian.org/new/ikos_3.2-1.html.

It can take months for that to be processed because of the strange license. The debian dev that put that there has been supernice about helping us; we just have to be very patient.

@xfiderek
Copy link
Contributor

@ivanperez-keera sorry, I could not reply earlier. Personally I am fine with whatever you decide. Since you are actively involved in IKOS dev, we have first-hand news and I find it hard to believe that it will slip out of your mind :).

@Bckempa Bckempa linked an issue Jan 26, 2024 that may be closed by this pull request
@ivanperez-keera ivanperez-keera merged commit cbf8146 into main Jan 26, 2024
3 checks passed
@ivanperez-keera ivanperez-keera deleted the dev-ikos branch January 26, 2024 20:31
eholum pushed a commit to eholum/docker that referenced this pull request Jun 9, 2024
Fix outdated usage and comments in `generate-repos.sh`
eholum pushed a commit to eholum/space-ros that referenced this pull request Jun 26, 2024
…en_text

Fix outdated usage and comments in `generate-repos.sh`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Docker images do not include ikos
3 participants