Skip to content
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

Installing LTS nodejs 16.13.1 (fix build on arm/v7) #17994

Closed
wants to merge 1 commit into from

Conversation

unlucio
Copy link

@unlucio unlucio commented Dec 15, 2021

Alpine's outdated node package doesn't play well with package-lock V2
This pulls the current long term support binaries from a more recent
alpine image rather than installing the registry's old one.

Why this patch: I tried to build a docker image for my raspberry pi for the better part of a day (both directly on the pi as well as on my computer using buildx). It kept falling all the time and stopping around Makefile's line 709 complaining some javascript callback cb() wasn't getting called. I tried to investigate the whereabouts of that cb() but I was unsuccessful.
At the same time, I noticed npm was consistently comparing about expecting a package-lock file V1 and instead of getting a V2, and ultimately blaming on its self the failure.
That prompted me to check what version of node was been installed and, with my great dismay, I was facing an ancient 14.*

At that point getting node to at least an LTS version was the sensible way to go. So here it is. Tested and working with buildx for linux/amd64, linux/arm64, linux/arm/v7.
This can potentially be great for other fellow raspberry pi users like me as (even with the fix) building something like gitea on a raspberry pi is not exactly a great experience 😅

@unlucio
Copy link
Author

unlucio commented Dec 15, 2021

And I see a "red light" but I've no idea what is complaining about what (or rather the bot doesn't give any log, which is less than ideal).
I see some bot failing to clone the repo, but I just forked this one soooooo I've no idea 🙃

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Dec 15, 2021
Alpine's outdated node package doesn't play well with package-lock V2
This pulls the current long term support binaries from a more recent
alpine image rather than installing the registry's old one.
@silverwind
Copy link
Member

I think this needs more investigation regarding this callback error. Node 14 should be fully supported to build and npm lockfile v2 should be fully backwards-compatible with npm 6.x to my understanding (it only raises a warning).

@silverwind
Copy link
Member

silverwind commented Dec 15, 2021

It works for me on x86:

docker run -it golang:1.17-alpine3.13
apk --no-cache add build-base git nodejs npm
git clone --depth 1 https://github.com/go-gitea/gitea.git && cd gitea
TAGS=bindata make build

A search on the cb() never called yields various results, some indicate bugs in npm v6, also see this. We could try updating npm itself before the build. Can you please try that?

npm install -g npm@latest

@codecov-commenter
Copy link

Codecov Report

Merging #17994 (743be42) into main (f58e687) will increase coverage by 0.05%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #17994      +/-   ##
==========================================
+ Coverage   45.28%   45.33%   +0.05%     
==========================================
  Files         820      820              
  Lines       90932    90932              
==========================================
+ Hits        41179    41225      +46     
+ Misses      43194    43154      -40     
+ Partials     6559     6553       -6     
Impacted Files Coverage Δ
modules/queue/workerpool.go 48.85% <0.00%> (-2.30%) ⬇️
modules/queue/queue_channel.go 95.00% <0.00%> (-1.67%) ⬇️
services/pull/pull.go 41.78% <0.00%> (-0.41%) ⬇️
models/issue_comment.go 53.04% <0.00%> (+0.56%) ⬆️
models/webhook/webhook.go 75.20% <0.00%> (+0.81%) ⬆️
modules/process/manager.go 82.94% <0.00%> (+1.55%) ⬆️
modules/git/git.go 40.81% <0.00%> (+2.04%) ⬆️
modules/git/repo_base_nogogit.go 86.84% <0.00%> (+2.63%) ⬆️
modules/git/utils.go 70.83% <0.00%> (+2.77%) ⬆️
modules/queue/queue_bytefifo.go 63.47% <0.00%> (+3.59%) ⬆️
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f58e687...743be42. Read the comment docs.

@unlucio
Copy link
Author

unlucio commented Dec 16, 2021

@silverwind X86 works for me as well, indeed I mentioned armv7 as the problem.
I just verified it still builds fine for x86 and arm64.

Anyway, I solved my problem and thought to share it or others that might have the same, if you want the code is there. Your choice.

cheers

@silverwind
Copy link
Member

I don't think we can accept such crude copying of /usr content. Updating npm only would be something I'd be willing to accept, thought.

@silverwind silverwind closed this Dec 16, 2021
@go-gitea go-gitea locked and limited conversation to collaborators Apr 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants