search

nodejs / postject

Easily inject arbitrary read-only resources into executable formats (Mach-O, PE, ELF) and use it at runtime.
Other
188 stars 14 forks source link

chore: update lief #23

Closed RaisinTen closed 2 years ago

RaisinTen commented 2 years ago

This change updates lief to https://github.com/lief-project/LIEF/commit/b183666a082d19ffc91ed0763f49e1d4f3814a59.

This contains https://github.com/lief-project/LIEF/pull/780, which is a requirement for https://github.com/postmanlabs/postject/pull/8.

Signed-off-by: Darshan Sen raisinten@gmail.com

dsanders11 commented 2 years ago

FWIW, I think in the future we should be cautious about updating LIEF when there's a large PR like #8 in-progress, since it may destabilize the PR and require more work to fix it back up, especially since the LIEF version being updated to isn't a stable release.

RaisinTen commented 2 years ago

We should have more tests to make sure that such a change isn't breaking any relevant component of postject. I don't think there's any other way to be sure otherwise. Also, this change was supposed to make the review process easier for that PR because it reduces a patch.

dsanders11 commented 2 years ago

Also, this change was supposed to make the review process easier for that PR because it reduces a patch.

It reduces a patch, but brings in N new commits from upstream, which is a mixed bag, and is the point I was making.

RaisinTen commented 2 years ago

Yes but the new commits shouldn't be problematic for us if it passes the test suite which is why I'm emphasizing on building a test suite ASAP that has tests for everything we care about.

dsanders11 commented 2 years ago

Yes but the new commits shouldn't be problematic for us

They can always be problematic for us in that they might break the build and require more work - even if it works on main. That's the reason that patch exists in the first place, those problems only appeared on the WebAssembly branch. So even if this PR didn't run into issues on main, it could still create more work on the WebAssembly branch due to those new commits, creating more work there.