nodejs / postject

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

build: stop building and testing on `alpine` and `debian` #26

Closed RaisinTen closed 2 years ago

RaisinTen commented 2 years ago

After https://github.com/postmanlabs/postject/pull/24 landed, we started building and testing on the linux executor, so there's no need to use the alpine and debian executors anymore.

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

bavulapati commented 2 years ago

@RaisinTen Can you mention the reason behind reducing the test matrix? I understand the reduction of build matrix, as we are not packaging postject differently on different executors.

RaisinTen commented 2 years ago

Same reason as the build matrix. I don't see the reason why we are running tests on alpine and debian when linux seems to be enough.

robertgzr commented 2 years ago

@RaisinTen as explained before in #24, I originally designed the matrix to have a broader coverage like this

while implementing linux support, I could've chosen to use glibc helpers for implementing the runtime access, but did not in favour of being able to support musl. having tests on both that ensure this continues to be the case was the main driver for testing on debian and alpine

I don't think postject should limit itself to the platforms covered by starship

RaisinTen commented 2 years ago

@robertgzr Ah so it's about musl, that sounds reasonable. Which one of debian and alpine uses musl?

RaisinTen commented 2 years ago

I don't think postject should limit itself to the platforms covered by starship

The thing is that starship is currently the only user of postject. Maybe we can add support for other platforms on need basis when we have users there?

dsanders11 commented 2 years ago

I don't think postject should limit itself to the platforms covered by starship

I agree with @robertgzr here.

The thing is that starship is currently the only user of postject. Maybe we can add support for other platforms on need basis when we have users there?

We want the project to be generic and forward looking, I don't see much of a downside to testing on more platforms. Either the tests pass and all is good, or they don't and there's something we should fix to keep the project working on as many platforms as possible.

RaisinTen commented 2 years ago

We want the project to be generic and forward looking

Yes, I'm fully supportive of that idea!

I don't see much of a downside to testing on more platforms.

But there is no upside either if there are no users of Postject on such platforms.

dsanders11 commented 2 years ago

But there is no upside either if there are no users of Postject on such platforms.

The benefit is we have test coverage and know it works on these platforms, we don't want to wait until there are users for those platforms to find out if they work. 🙂

RaisinTen commented 2 years ago

During the discussion in the Starship meeting I learnt that alpine, which is musl-based is significantly popular. I didn't consider that while sending this PR, so I'll remove the alpine related changes. But how about the debian part? How popular is that w.r.t linux and alpine @robertgzr?