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

Update for Ubuntu 24.04 #336

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

pjonsson
Copy link

@pjonsson pjonsson commented Dec 2, 2024

Switch the Dockerfile to using
the ubuntu user/group which is
required for building after
davidfrantz/base_image#4
has been merged.

Also remove the WORKDIR since that
is the last thing set by the Dockerfile
for davidfrantz/base.

Include paths for some dependencies
have changed, so update the Makefile
with those paths as well.

@pjonsson
Copy link
Author

pjonsson commented Dec 2, 2024

I have built the image, and since that prints a bunch of messages from various force-tools, it's not obviously broken, but I would appreciate help with testing this. The CI tests will obviously fail on this in its current state since the Ubuntu 24.04 base image isn't present yet.

@davidfrantz
Copy link
Owner

Thanks @pjonsson,

as discussed in the base repository, I appreciate the addition. I will merge this PR at some point, but will test first.

This will take some time, but will happen eventually

Thanks,
David

@pjonsson
Copy link
Author

@davidfrantz would you like me to rebase on latest develop and add a commit that changes the FROM line to use the ubuntu24 tag instead of latest?

Switch the Dockerfile to using
the ubuntu user/group which is
required for building after
davidfrantz/base_image#4
has been merged.

Also remove the WORKDIR since that
is the last thing set by the Dockerfile
for davidfrantz/base.

Include paths for some dependencies
have changed, so update the Makefile
with those paths as well.
@pjonsson pjonsson force-pushed the upgrade-ubuntu24 branch 2 times, most recently from bd4e4bb to 8f3a51b Compare December 23, 2024 11:35
@pjonsson
Copy link
Author

I rebased on top of latest develop and added a commit that sets the FROM to use the Ubuntu 24.04 version of the base image.

I also added a workaround that sets the home directory to world-writable so the CI tests could pass, but I wouldn't recommend having a world-writable home directory for security reasons. If you want to avoid that, there's a way to re-uid/re-gid the user inside the container upon startup (example here: contiki-ng/contiki-ng#2309, but you also want contiki-ng/contiki-ng#2822 on top of that to handle signal delivery inside the container properly).

It's not related to this PR, but when I looked at the CI failure I noticed that Force is compiled with g++ -std=c++11 which I guess means it's a C++ program. All the source code files have a .c suffix rather than the C++ standard suffixes (.cc/.cxx/.cpp) though, so I was surprised that it's C++. I've never done anything like that, but my guess is that you will save yourself some trouble with fighting tools if you rename the C++ source files so their suffixes match the language they are written in.

I also saw a bunch of compiler warnings. I have not looked at the code, but my guess is that the differing sign warnings are unlikely to be a problem in practice, but the stringop-warnings looked like they should be addressed.

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

Successfully merging this pull request may close these issues.

2 participants