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

nodejs: merge with nodejs-lts #40106

Closed
wants to merge 2 commits into from
Closed

Conversation

paper42
Copy link
Member

@paper42 paper42 commented Oct 22, 2022

Nodejs versioning says that every even release (12, 14, 16, 18) is an LTS release. The nodejs package currently uses version 16 which is a supported LTS version, nodejs-lts uses version 12 which is EOL and very old. Many packages use nodejs-lts for building, but then depend on the nodejs virtual package which defaults to nodejs, many packages don't work with old nodejs-lts and people couldn't have both installed. If we need to, we can always split nodejs-lts again, but right now I don't see a reason to do so. Alpine merged their nodejs-lts package to nodejs and provides nodejs-current for the latest version for development.

TODO:

  • fix chronograf build with nodejs 16 (probably with an update)

Testing the changes

  • I tested the changes in this PR: briefly

@paper42 paper42 changed the title nodejs: update to 16.18.0, merge with nodejs-lts nodejs: merge with nodejs-lts Nov 22, 2022
@leahneukirchen
Copy link
Member

Hmm, I think we had the -lts because some packages needed a really old Node (but I thought, like, Node 8). If that's not the case anymore, we can just drop it.

@akierig
Copy link
Contributor

akierig commented Dec 15, 2022

all in favor of updating node. I can't update Signal-Desktop's latest version because it requires node ≥ 16.16.0. Happy to help if I can, @paper42.

@paper42 paper42 force-pushed the node-lts-16 branch 2 times, most recently from 3b5abe0 to 62e73e5 Compare December 18, 2022 11:08
@dkwo
Copy link
Contributor

dkwo commented Feb 4, 2023

Can I rely on this PR for openssl3 update #37681 ? nodejs-lts was failing to build with openssl3, perhaps being too old.

@paper42
Copy link
Member Author

paper42 commented Feb 4, 2023

No, chronograf still doesn't build with nodejs 16, this will be merged when it does, but I have not been able to fix it so far.

@dkwo
Copy link
Contributor

dkwo commented Feb 4, 2023

Have you tried updating it to 1.10.0 ?

@unrealjo unrealjo mentioned this pull request Mar 1, 2023
19 tasks
@paper42
Copy link
Member Author

paper42 commented Mar 1, 2023

Have you tried updating it to 1.10.0 ?

I did, but there was an error and since I don't know nodejs, I ended there. 1.10.0 should support nodejs 16 though

@dkwo
Copy link
Contributor

dkwo commented Mar 7, 2023

@paper42 I was able to build it with

diff --git a/srcpkgs/chronograf/template b/srcpkgs/chronograf/template
index 5dfcc1c3ef..087208b040 100644
--- a/srcpkgs/chronograf/template
+++ b/srcpkgs/chronograf/template
@@ -1,18 +1,18 @@
 # Template file for 'chronograf'
 pkgname=chronograf
-version=1.9.4
+version=1.10.0
 revision=1
 build_style=go
 go_import_path="github.com/influxdata/${pkgname}"
 go_package="${go_import_path}/cmd/chronograf"
 go_ldflags="-X main.version=${version}"
-hostmakedepends="dep go-bindata nodejs-lts yarn"
+hostmakedepends="dep go-bindata nodejs yarn python3"
 short_desc="Open source monitoring and visualization UI for the TICK stack"
 maintainer="Michael Aldridge <maldridge@voidlinux.org>"
 license="AGPL-3.0-or-later"
 homepage="https://www.influxdata.com/time-series-platform/chronograf/"
 distfiles="https://github.com/influxdata/${pkgname}/archive/${version}.tar.gz"
-checksum=ff294f25a9de57140024b9953992c1a4d79ec88167ad28435645d888a0096c27
+checksum=4c9ec541a77314b11f23f2eff1394568ea9180f1f3cc3f098cb3e7977dbfd7a5
 
 system_accounts="_chronograf"
 _chronograf_homedir="/var/lib/${pkgname}"
@@ -23,13 +23,13 @@ case "$XBPS_TARGET_MACHINE" in
        ppc*) broken="ftbfs in some js module" ;;
 esac
 
-pre_build() {
-       cd $wrksrc/ui
-       yarn install
-       export PATH=$PATH:${wrksrc}/ui/node_modules/.bin
+do_build() {
+       go get -p "$XBPS_MAKEJOBS" -x -tags "${go_build_tags}" -ldflags "${go_ldflags}" github.com/influxdata/chronograf
+       CFLAGS="$CFLAGS -fPIC" CXXFLAGS="$CXXFLAGS -fPIC" make 
+}
 
-       cd $wrksrc
-       make assets
+do_install() {
+       go install github.com/influxdata/chronograf/cmd/chronograf
 }
 
 post_install() {

cc mantainer @the-maldridge
it may even build with nodejs 18 from #41239 , but I have not checked.

@paper42
Copy link
Member Author

paper42 commented Mar 7, 2023

@dkwo great, could you open a PR with that change?

@dkwo
Copy link
Contributor

dkwo commented Mar 7, 2023

done #42644

@dkwo
Copy link
Contributor

dkwo commented May 16, 2023

@paper42 can this be merged, now that chronograf is fixed?

nodejs 16 is an LTS version and nodejs-lts version 12 is EOL
nodejs-lts is now merged to nodejs
@paper42 paper42 marked this pull request as ready for review June 2, 2023 19:24
@paper42
Copy link
Member Author

paper42 commented Jun 3, 2023

merged in 0fbf636 and 78574d7

@paper42 paper42 closed this Jun 3, 2023
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.

None yet

4 participants