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

Fix NX Compatibility #35

Merged
merged 2 commits into from
Jan 26, 2023
Merged

Fix NX Compatibility #35

merged 2 commits into from
Jan 26, 2023

Conversation

superm1
Copy link
Member

@superm1 superm1 commented Jan 25, 2023

It was supposed to be set, but an argument was missing. This sets it and makes sure it's set.

@superm1 superm1 requested a review from hughsie January 25, 2023 22:30
@hughsie
Copy link
Member

hughsie commented Jan 25, 2023

I don't think we have genpeimg in fedora -- @vathpela -- help?

@superm1
Copy link
Member Author

superm1 commented Jan 25, 2023

Ahah, and the arch failure is the reason I wanted to flip the default. I would have had no idea!

@ArchangeGabriel, I think we're missing genpeimg in arch. I looked through https://archlinux.org/packages/ but couldn't find it. Can you help?

We need it to make the default NX compatibility.

@superm1
Copy link
Member Author

superm1 commented Jan 25, 2023

I can alternatively come up with a way to do this with python3-pefile if that's more readily available.

@superm1
Copy link
Member Author

superm1 commented Jan 25, 2023

So I don't lose it in my history, here is how to do it with python3-pefile:

import pefile
pe = pefile.PE(file)
pe.OPTIONAL_HEADER.DllCharacteristics |= pefile.DLL_CHARACTERISTICS["IMAGE_DLLCHARACTERISTICS_NX_COMPAT"]
pe.write('new.efi')

@superm1
Copy link
Member Author

superm1 commented Jan 25, 2023

OK, I think this is maximum flexibility now. Uses genpeimg as a preference and python3-pefile if not present. If both are missing fail the build. Net outcome should be we're always NX compatible.

Copy link
Member

@hughsie hughsie left a comment

Choose a reason for hiding this comment

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

Yell when you want a new tarball.

@superm1 superm1 merged commit 81164c8 into main Jan 26, 2023
@superm1 superm1 deleted the mlimonci/nx branch January 26, 2023 12:17
@superm1
Copy link
Member Author

superm1 commented Jan 26, 2023

Yell when you want a new tarball.

I think now. Everyone is about to spin shim so we should make them spin this too.

@ArchangeGabriel
Copy link
Contributor

Ahah, and the arch failure is the reason I wanted to flip the default. I would have had no idea!

@ArchangeGabriel, I think we're missing genpeimg in arch. I looked through https://archlinux.org/packages/ but couldn't find it. Can you help?

We need it to make the default NX compatibility.

Where is it supposed to be coming from? (I only see mingw related things by searching that on the net).

Also note that @FFY00 and @freswa are also maintaining fwupd in Arch now (and actually more actively than me!). ;)

@superm1
Copy link
Member Author

superm1 commented Jan 26, 2023

Where is it supposed to be coming from? (I only see mingw related things by searching that on the net).

Yeah it comes from a mingw package as I can tell. What we ended up doing instead was making it a dependency for either that or python3-pefile, which I did manage to find in the arch repos. We added it to CI and confirmed it fixes the Arch build.

Also note that @FFY00 and @freswa are also maintaining fwupd in Arch now (and actually more actively than me!). ;)

OK, thanks for heads up! FYI - Once @hughsie tags a build you guys will need to bump your PKGBUILD for fwupd-efi to include python-pefile.

@FFY00
Copy link

FFY00 commented Jan 26, 2023

Yeah, no worries, just ping me here or on IRC when that happens, if possible. If I don't act timely, please ping me again, as ADHD sometimes lets my brain get hijacked by other things 😅

archlinux-github pushed a commit to archlinux/svntogit-community that referenced this pull request Jan 26, 2023
Upstream mentioned [0] that upcoming releases will python-pefile to build. It
has been verified by their CI already.

[0] fwupd/fwupd-efi#35


git-svn-id: file:///srv/repos/svn-community/svn@1388364 9fca08f4-af9d-4005-b8df-a31f2cc04f65
archlinux-github pushed a commit to archlinux/svntogit-community that referenced this pull request Jan 26, 2023
Upstream mentioned [0] that upcoming releases will python-pefile to build. It
has been verified by their CI already.

[0] fwupd/fwupd-efi#35

git-svn-id: file:///srv/repos/svn-community/svn@1388364 9fca08f4-af9d-4005-b8df-a31f2cc04f65
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants