michaelmcandrew / civicrm-buildkit-docker

This is a read only copy. Please make PRs here: https://lab.civicrm.org/michaelmcandrew/civicrm-buildkit-docker
https://lab.civicrm.org/michaelmcandrew/civicrm-buildkit-docker
GNU Affero General Public License v3.0
40 stars 31 forks source link

Node.js is outdated #68

Open wmortada opened 3 years ago

wmortada commented 3 years ago

When building the image, I get the following notice:

================================================================================
================================================================================

                              DEPRECATION WARNING                            

  Node.js 10.x is no longer actively supported!

  You will not receive security or critical stability updates for this version.

  You should migrate to a supported version of Node.js as soon as possible.
  Use the installation script that corresponds to the version of Node.js you
  wish to install. e.g.

   * https://deb.nodesource.com/setup_12.x — Node.js 12 LTS "Erbium"
   * https://deb.nodesource.com/setup_14.x — Node.js 14 LTS "Fermium" (recommended)
   * https://deb.nodesource.com/setup_16.x — Node.js 16 "Gallium"

  Please see https://github.com/nodejs/Release for details about which
  version may be appropriate for you.

  The NodeSource Node.js distributions repository contains
  information both about supported versions of Node.js and supported Linux
  distributions. To learn more about usage, see the repository:
    https://github.com/nodesource/distributions

================================================================================
================================================================================

I've tracked it down to this line:

RUN curl -sL https://deb.nodesource.com/setup_10.x | bash - 

Presumably upgrading the version to 14 will fix this? I'll give it a go.

wmortada commented 3 years ago

Hmm. This is consistent with buildkit so perhaps that is out of date too?

https://github.com/civicrm/civicrm-buildkit/blob/fee9a80114c10f541a43a57179664fb8dc19fe8b/bin/civi-download-tools#L502

wmortada commented 3 years ago

It appears to work fine with version 14. I'll add a commit to update this.

michaelmcandrew commented 3 years ago

@wmortada - I am wondering if we can rely on civi-download-tools to download node - see https://lab.3sd.io/tools/civicrm-buildkit-docker/-/commit/3b7531d7bf221021ac44756ab5d3cd2919bf22c9.

michaelmcandrew commented 3 years ago

It appears that we can rely on civi-download-tools to download node so am suggesting that we remove our manual downloading of node, and wait for it to be fixed upstream in line with the general philosophy of this project :)

Feel free to close if you agree else we can carry on discussing :)

wmortada commented 3 years ago

That makes sense to me. I was wondering the same. It did seem like duplication. I haven't had a chance to test it yet though. Are you going to commit it to this repository (i.e. GitHub rather than GitLab)?

michaelmcandrew commented 3 years ago

So according to https://github.com/michaelmcandrew/civicrm-buildkit-docker/issues/73 there is an issue with this method at the mo. I suspect it is solvable though.

I am now just committing new code to the gitlab repo. You should be able to update the remote with something like

git remote set-url origin ssh://git@lab.3sd.io:2222/tools/civicrm-buildkit-docker.git

michaelmcandrew commented 3 years ago

OK so actually we cannot really rely on civi-download-tools to install node.

There is a civi-download-tools --full mode that at at first glance, I thought would support this but full mode does stuff like use the sury.org php packages and this approach is (philosophically and technically) incompatible with using the `php:7.3-apache-buster' docker image.

Switching our docker image to debian:buster and letting civi-download-tools handle all of the dependencies is potentially a smart idea (and civi-download tools does do things like download php and apache so arguably it is the way to do if we want to offload as much as possible upstream, but it would be a significant change and require a fair amount of testing / is not to be rolled out lightly.

michaelmcandrew commented 3 years ago

Here is a new commit that fixes the issue: https://lab.3sd.io/tools/civicrm-buildkit-docker/-/commit/ad17001c8161dcd05e597e9f8d48c85e8b0a1974 - it is basically the same as what @wmortada did.

A new image on dockerhub should be generated overnight which should work fine, fingers crossed.

wmortada commented 3 years ago

I am now just committing new code to the gitlab repo.

Does this mean you are moving development to GitLab? I'm happy with either, but wondering where new issues and PRs should be raised?

michaelmcandrew commented 3 years ago

Ideally all on Gitlab but we don't have user sign up there at the mo and it is annoying for people to create another account so for now, am kicking the can down the road and keeping issues here...