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

Feature/debian12 #502

Merged
merged 12 commits into from
Dec 17, 2024
Merged

Feature/debian12 #502

merged 12 commits into from
Dec 17, 2024

Conversation

paulnoalhyt
Copy link
Collaborator

One sentence summary of this PR (This should go in the CHANGELOG!)
After a few attempts (#467 and #494), another try at moving to Debian 12.

Link to Related Issue(s)
#495

Please describe the changes in your request.
This time updating base Docker image to python3.9 and installing toolchains from files instead of package manager. Had to update Lief (old version was causing Binary Ninja to segfault). Fixed tests along the way.

Anyone you think should look at this, specifically?
@whyitfor

This was referenced Dec 4, 2024
@whyitfor
Copy link
Contributor

whyitfor commented Dec 9, 2024

@paulnoalhyt, can you work through the merge conflicts on this one?

Copy link
Member

@rbs-jacob rbs-jacob left a comment

Choose a reason for hiding this comment

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

This mostly looks good! It's really good that we're finally pinning very specific toolchain versions. I added a couple of very minor suggestions below.

ofrak_patch_maker/Dockerstub Outdated Show resolved Hide resolved
ofrak_core/test_ofrak/components/test_symbolic_analysis.py Outdated Show resolved Hide resolved
ofrak_core/test_ofrak/components/test_uefi_component.py Outdated Show resolved Hide resolved
Copy link
Collaborator Author

@paulnoalhyt paulnoalhyt left a comment

Choose a reason for hiding this comment

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

Applied all suggestions.

Copy link
Contributor

@whyitfor whyitfor left a comment

Choose a reason for hiding this comment

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

Looks good! See a few comments

build_image.py Show resolved Hide resolved
build_image.py Outdated Show resolved Hide resolved
ofrak_core/Dockerstub Show resolved Hide resolved
Copy link
Contributor

@whyitfor whyitfor left a comment

Choose a reason for hiding this comment

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

LGTM!

@whyitfor whyitfor dismissed rbs-jacob’s stale review December 17, 2024 17:18

I have validated that all requested changes made it into the PR.

@whyitfor whyitfor merged commit 935ba58 into master Dec 17, 2024
4 checks passed
@whyitfor whyitfor deleted the feature/debian12 branch December 17, 2024 18:11
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.

3 participants