Skip to content

Ship esbuild binaries for all platforms in tarball #12

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

Merged
merged 2 commits into from
Feb 15, 2021

Conversation

CL-Jeremy
Copy link

Tested working on macOS and Windows without Internet connection, both for building and for releasing.

Note that testing with npx shows that even without binary file installed into node_modules/esbuild/bin and node_modules/.bin, having a native subpackage installed would suffice in most cases.

And esbuild does not provide a mips64 binary, so good that we've already deprecated that.

I won't be able to do further testing as I happened to have a workplace partition with useful data wiped out on a SSD and thus have to deal with file recovery for that machine first. Sorry for that. Any improvement to the Makefile is highly appreciated.

@CL-Jeremy CL-Jeremy force-pushed the esbuild branch 3 times, most recently from 485cd00 to 4e95592 Compare February 6, 2021 23:11
@CL-Jeremy
Copy link
Author

CL-Jeremy commented Feb 6, 2021

Well, it turns out testing was absolutely necessary.

Not only was my guessing completely wrong, but the part for creating tarball itself was already faulty, at least under macOS/with bsdtar, such that the resulting tarball would exclude all dist subfolders embedded in node_modules, including that installed by npm, making the vendoring incomplete and thus unusable when offline. Adding the leading caret to the rule resolves this.

I'm now making use of caching for the postinstall step of esbuild and letting that script do the job of installing the correct binary. The cache, when source is pulled from the tarball, is only deleted when release-sources is executed again. Maybe it's better to delete that in the postinstalling step as well:

@@ -647,7 +647,8 @@ node-modules: .node_modules.ready
        npm install --no-save
        @if [ ! -f node_modules/esbuild/bin/esbuild ]; then \
                cd node_modules/esbuild; \
-               npm run postinstall; \
+               npm run postinstall && \
+               rm -rf .*; \
                cd ../..; \
        fi
        @[ $$? -eq 0 ] && touch .node_modules.ready

Edit: it now includes the above change.

@silverwind
Copy link

See go-gitea#14578 (comment). I'm in favor of just excluding node_modules in the tarballs instead.

@CL-Jeremy
Copy link
Author

Changed to tackle the problem with npm-cache. Code is a lot less obscure now. No more hacky workarounds. Just clear logic.

The packages that are manually added, besides esbuild binaries, are: less@3 (for fomantic), fsevents@1 and fsevents@2 (for macOS file system operations).

@lunny
Copy link
Owner

lunny commented Feb 13, 2021

How did you resolve the problem to build Gitea without network?

@CL-Jeremy
Copy link
Author

How did you resolve the problem to build Gitea without network?

CL-Jeremy@31c2c97#diff-76ed074a9305c04054cdebb9e9aad2d818052b07091de1f20cad0bbac34ffb52R641-R650

By doing this, I'm telling npm to use a local cache (.npm-cache of project directory), and cache all the packages that are to be downloaded from a fresh npm --install --no-save, plus the platform-specific binaries mentioned one comment above. Together they make around 800 packages. Then I go on to also cache packages for building Fomantic UI, which are also like 700 packages altogether, but I choose to put the node_nodules folder for that build in another location, namely web_src/fomantic, so Fomantic could be built completely offline as well (as for now, Fomantic requires less@3 due to one of its dependencies not yet supporting Less 4, hence the separation). When releasing, node_modules at both locations are excluded, leaving only .npmrc for use on the target platform. There, npm will take care of all the platform-specific jobs, as if npm install was never run on another platform. It's a hacky way to make sure no package needs to be downloaded from the Internet on every platform.

@lunny lunny merged commit 28479bb into lunny:lunny/esbuild Feb 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants