feat: allow to use custom LLRT binary with llrtBinaryPath

[georeeve]

Adds a llrtCustomBinaryDirectory property to allow users to use their own LLRT bootstrap binary.

This could be implemented in a few different ways. I tried to keep as much of the existing solution as possible:
If llrtCustomBinaryDirectory is set, it looks for a "bootstrap" binary in that directory, and errors if it cannot find one.
If llrtCustomBinaryDirectory is not set, the binary is downloaded from GitHub and cached in the .tmp directory as before.

[tmokmss]

@georeeve hi thanks for the PR! to clarify, what is the usecase for this feature? (Why do you want to configure the cache directory?) Thanks.

[georeeve]

@tmokmss Hi, sorry if I didn't make it clear in the docs. My use case was that I wanted to test out a commit from LLRT's main branch, so I built the bootstrap binary locally. I then wanted to deploy this in my stack. I could have tricked LLRT CDK into deploying the binary by saving it as the .tmp/llrt/v0.3.0-beta/arm64/standard/bootstrap file, but thought that that was a bit of a hack and it'd make more sense to make a specific property for it.

So with this PR, you can set llrtCustomBinaryDirectory to .llrt for example, and then you only need to save it as .llrt/bootstrap. I would've normally created an issue first and then a PR, but given that I had changed the code anyway so that I could test the LLRT commit, I thought I'd just raise a PR. No worries if you don't think it's a necessary feature though.

[tmokmss]

@georeeve Thanks for the detailed clarification! Actually I have some concerns about this proposed feature:

1. The bootstrap cache path becomes the same regardless of architecture, version, or binary type. It can confuse some users when they naively set llrtCustomBinaryDirectory prop.
2. This feature can prevent future changes in the caching strategy. I'd like to hide this part as implementation detail that users don't need to care.

Instead, I'd just overwrite .tmp/llrt/**/bootstrap file for testing purpose as you originally did. The path may change in the future indeed, but you can then adjust the binary path to overwrite.

[georeeve]

Those concerns are valid, I still think there's a use case for some projects to want to use a custom binary permanently in their project, perhaps if they apply some specific patches on top of LLRT. Would you accept a llrtCustomBinary property which specifies a file path to a specific bootstrap binary, and use this instead of altering the cache directory logic? We could then leave the caching strategy alone, and completely bypass the afterBundling step if llrtCustomBinary is provided.

[tmokmss]

I see. then let's add the feature! I'll take a close look at the PR tomorrow so just a moment please 🙏

[georeeve]

@tmokmss Sorry for the delay, I've added the llrtBinaryPath prop now, let me know how it looks.

[tmokmss]

Thank you @georeeve! I'll confirm the detailed behavior in my local env tomorrow. I think we can merge this after that :)

[tmokmss]

Not sure if this works well in some edge cases. Does it work whenforceDockerBundling: true and/or depsLockFilePath: 'some/child/packege-lock.json'? I'll test those cases later.

[tmokmss]

Also we may need an integration test to maintain this feature. I'll add one along the above test.
edit) @georeeve If you are willing to add it, that is totally welcome though!

[georeeve]

Good spot, I'm not sure if it would work with forceDockerBundling. I imagine you would have to mount the directory somehow but I don't know if there's an option for that?

[georeeve]

@tmokmss I can take a look at the integration tests, does it just run with yarn integ-runner?

[tmokmss]

yes. you may need to run yarn build for tsc though.

[tmokmss]

Confirmed that as long as llrtBinaryPath is relative to the projectRoot directory, it works even with Docker. I added that information to jsdoc.

[tmokmss]

@georeeve have you tried to support absolute paths as well? Ideally we want to support them but I suspect it is difficult.

[georeeve]

I haven't, I can try but I'm not able to test the behaviour on Windows, which I imagine would be different. If Windows requires Docker builds then I suppose that's a different problem entirely though.

[tmokmss]

@georeeve I made some minor modification to the code and doc, as well as adding an integ-test. If you are okay with these changes, I'll merge this! :)

[georeeve]

@tmokmss LGTM, thanks for doing that.