[nodejs planner] InputFiles should take paths relative to devbox.json, and not combined with srcDir

[savil]

Summary

This should fix a consequence of a change introduced in #200 to make the devbox.srcDir be an absolute path.
In the BuildPlanners, we need the paths to be relative so they are inside the "docker context" and
can be used in Dockerfiles.

New Fix:
I think the problem is actually scoped to just the NodeJSPlanner. Within NodeJSPlanner, the inputFiles were being set using filepath.Join(srcDir, <filename>) when instead it should be just <filename>, or more pedantically path/of/filename/from/devbox-json.

This is because the docker-context is the directory-of-devbox-json, and any files COPY --link'd should be specified with paths relative to this docker-context.

I inspected git grep InputFiles and it seems NodeJSPlanner is the only one with this issue.

I also went back in history to a commit from Monday, built the binary and found that the following would fail due to this reason:

> cd testdata/nodejs
> devbox build nodejs-18

It works now with this fix.

How was it tested?

in www.jetpack.io repo,

1. did devbox build and docker run -p 3000:3000 --expose 3000 -ti devbox and could open the website in localhost:3000
2. did cd root-of-www.jetpack.io and did devbox build www && docker run -p 3000:3000 --expose 3000 -ti devbox. Works!
3. did mkdir -p root-of-www.jetpack.io/www/fake-dir && cd root-of-www.jetpack.io/www/fake-dir and then: devbox build ../ && docker run -p 3000:3000 --expose 3000 -ti devbox. Works! Also, works with just devbox build i.e. omitting ../ argument (due to [config] look for the devbox.json file in parent directories as well #200)

For NodeJS testdata in testdata/nodejs:

1. cd testdata/nodejs and devbox build nodejs-18 && docker run devbox. Works!
2. cd testdata/nodejs/nodejs-18 and devbox build && docker run devbox. Works!
3. cd testdata/nodejs/nodejs-18/fake-dir and devbox build && docker run devbox. Works!

in devbox repo,
did devbox build and docker run devbox with:

• testdata/rust/rust-stable
• testdata/python/pip-example

[savil]

Current dependencies on/for this PR:

• main
  • PR [nodejs planner] InputFiles should take paths relative to devbox.json, and not combined with srcDir #212 👈

This comment was auto-generated by Graphite.

[LucilleH]

👍
Just to confirm, this works with devbox build <absolute_path>, devbox build <relative_child_path>, devbox build <relative_parent_path>, and devbox build?

[savil]

👍
Just to confirm, this works with devbox build <absolute_path>, devbox build <relative_child_path>, devbox build <relative_parent_path>, and devbox build?

I'll test these to verify, but conceptually these should work because we normalize all of these by using the absolute-path in devbox.Open() when initializing devbox.srcDir.

EDIT: ugh, no this will still fail 😩

[savil]

This has a bug: using devbox build <path> can sometimes fail. Will re-submit for review after I fix it.

[LucilleH]

Thanks for fixing!

[loreto]

For a follow up PR: What can we do to catch these automatically? (and avoid a regression)

[savil]

For a follow up PR: What can we do to catch these automatically? (and avoid a regression)

Good question. I was thinking about this as well. One idea is to have a validatePlan function in planner, which can have validation rules. One validationRule can check that the inputFiles are not absolute paths.

We could run this validatePlan in devbox_test or even every time planner.BuildPlan() and planner.ShellPlan() is called. My preference is for doing it in devbox_test and ensuring we have good test coverage of various scenarios for the planners.