Skip to content

Avoid spurious libgcc-s1 Debian package dependency #5877

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

Merged

Conversation

tautschnig
Copy link
Collaborator

Marked as Draft (and do-not-merge, do-not-review), includes extra debug output.

Do not add a strict dependency on gcc, which in a GitHub container isn't
necessarily the same version as in a pristine Ubuntu system.

Fixes: #5875

  • Each commit message has a non-empty body, explaining why the change was made.
  • n/a Methods or procedures I have added are documented, following the guidelines provided in CODING_STANDARD.md.
  • n/a The feature or user visible behaviour I have added or modified has been documented in the User Guide in doc/cprover-manual/
  • Regression or unit tests are included, or existing tests cover the modified code (in this case I have detailed which ones those are in the commit message).
  • n/a My commit message includes data points confirming performance improvements (if claimed).
  • My PR is restricted to a single feature or bugfix.
  • n/a White-space or formatting changes outside the feature-related changed lines are in commits of their own.

@codecov
Copy link

codecov bot commented Mar 1, 2021

Codecov Report

Merging #5877 (7f213db) into develop (6779f39) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #5877   +/-   ##
========================================
  Coverage    72.90%   72.90%           
========================================
  Files         1425     1425           
  Lines       154282   154282           
========================================
  Hits        112485   112485           
  Misses       41797    41797           

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 7f998c4...8514130. Read the comment docs.

@@ -375,6 +375,7 @@ jobs:
run: |
sudo apt-get update
sudo apt-get install --no-install-recommends -y g++ flex bison cmake ninja-build maven jq libxml2-utils dpkg-dev ccache
sudo apt-get --purge remove -s libgcc-s1

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh, that seems screwy? So the github actions ubuntu-18.04 container isn't necessarily actually ubuntu-18.04?

@hannes-steffenhagen-diffblue
Copy link
Contributor

For curiosities sake, how did you detemine this was the issue?

@kroening
Copy link
Member

kroening commented Mar 1, 2021

The cleaner answer here could be do build that package in a docker container (Github actions does this). The problem with the proposed tweak is that Github may well choose to make further changes to their standard image, and then the issue is back.

@hannes-steffenhagen-diffblue
Copy link
Contributor

@tautschnig Following @kroening's comment here, I believe the correct thing to do would be to specify this running on the ubuntu:18.04 docker image via the container attribute, then we don't have to remove packages which seems slightly sketchy anyway.

TBH I didn't realise that the actions images had so much nonstandard stuff in them when we came up with these actions.

@tautschnig tautschnig force-pushed the no-libgcc-s1-dep branch 21 times, most recently from 672733e to 3d3eed2 Compare March 3, 2021 09:54
@tautschnig tautschnig force-pushed the no-libgcc-s1-dep branch 3 times, most recently from 7f213db to 4fb7ebc Compare March 3, 2021 10:15
GitHub actions have GCC 10 available, which resulted in GCC libraries
being upgraded as well. Forcibly downgrade this to Bionic's (18.04)
native version, removing GCC 9 and 10 along the way.

Using `container: ubuntu:18.04` would potentially be a cleaner solution,
but our use of submodules requires Git >= 2.18, which isn't in Ubuntu
18.04. So we'd need to start mixing version along a different path.

Fixes: diffblue#5875
@tautschnig
Copy link
Collaborator Author

@tautschnig Following @kroening's comment here, I believe the correct thing to do would be to specify this running on the ubuntu:18.04 docker image via the container attribute, then we don't have to remove packages which seems slightly sketchy anyway.
[...]

I tried this avenue, but then our use of submodules requires git 2.18, which isn't in Ubuntu 18.04. Thus sticking with the force-downgrade option for now. Marking the PR as ready-for-review as the logs confirmed that libgcc-s1 no longer is among the dependencies being generated.

@tautschnig tautschnig marked this pull request as ready for review March 3, 2021 11:28
@tautschnig tautschnig requested a review from a team as a code owner March 3, 2021 11:28
Copy link
Contributor

@NlightNFotis NlightNFotis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@thomasspriggs
Copy link
Contributor

@tautschnig Following @kroening's comment here, I believe the correct thing to do would be to specify this running on the ubuntu:18.04 docker image via the container attribute, then we don't have to remove packages which seems slightly sketchy anyway.
[...]

I tried this avenue, but then our use of submodules requires git 2.18, which isn't in Ubuntu 18.04. Thus sticking with the force-downgrade option for now. Marking the PR as ready-for-review as the logs confirmed that libgcc-s1 no longer is among the dependencies being generated.

Can you further explain what aspects of submodules aren't working with git 2.17? I am using ubuntu 18.04 locally and I haven't seen any submodule related issues.

@tautschnig
Copy link
Collaborator Author

@tautschnig Following @kroening's comment here, I believe the correct thing to do would be to specify this running on the ubuntu:18.04 docker image via the container attribute, then we don't have to remove packages which seems slightly sketchy anyway.
[...]

I tried this avenue, but then our use of submodules requires git 2.18, which isn't in Ubuntu 18.04. Thus sticking with the force-downgrade option for now. Marking the PR as ready-for-review as the logs confirmed that libgcc-s1 no longer is among the dependencies being generated.

Can you further explain what aspects of submodules aren't working with git 2.17? I am using ubuntu 18.04 locally and I haven't seen any submodule related issues.

See https://github.com/diffblue/cbmc/runs/2021105147 for one of the past logs. That says:

The repository will be downloaded using the GitHub REST API
To create a local Git repository instead, add Git 2.18 or higher to the PATH
Error: Input 'submodules' not supported when falling back to download using the GitHub REST API. To create a local Git repository instead, add Git 2.18 or higher to the PATH.

@hannes-steffenhagen-diffblue
Copy link
Contributor

@tautschnig That seems like an issue with the github action we use for fetching the code. I don't think that's a real blocker for building on 18.04 proper, worst case we can just work around that.

@tautschnig
Copy link
Collaborator Author

@tautschnig That seems like an issue with the github action we use for fetching the code. I don't think that's a real blocker for building on 18.04 proper, worst case we can just work around that.

Up to you. Can I suggest we put a fix in place before merging #5883 today? This might be merging this PR and doing a cleaner solution later on, or someone making the container solution work today.

@tautschnig tautschnig mentioned this pull request Mar 4, 2021
7 tasks
@hannes-steffenhagen-diffblue
Copy link
Contributor

@tautschnig seems good to me, plan is to merge this and put building in docker in the mid-term

@hannes-steffenhagen-diffblue hannes-steffenhagen-diffblue merged commit 65fac51 into diffblue:develop Mar 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ubuntu 18.04 deb depends on libgcc-s1 which isn't in 18.04, it should depend on libgcc1 instead
5 participants