Skip to content
This repository has been archived by the owner on Jan 15, 2024. It is now read-only.

[RFC] refactor the installer for 2.0 #141

Closed
Ma27 opened this issue Mar 29, 2016 · 3 comments
Closed

[RFC] refactor the installer for 2.0 #141

Ma27 opened this issue Mar 29, 2016 · 3 comments
Assignees
Milestone

Comments

@Ma27
Copy link
Collaborator

Ma27 commented Mar 29, 2016

Current status

The installer

The current implementation provides the possibility of installing mutiple nodejs instances.
It installs instances by using symlinks like node-v5.8.0 and creates one default version which is accessible via node. The same approach should be driven with npm executables, but didn't work properly as described in bug #94 .
Furthermore switching the default instance is quite diffcult as parts of the manifest instantiating this module need to be rewritten:

class { '::nodejs':
    version => 'x.y',
}

::nodejs::install { 'vY.X':
    version => 'y.x',
}

If the instance with version y.x should be the default one now, you would have to rewrite the parameters of the class and the install resources while changing a default_version parameter would be simpler.

The approach contained such issues as described above and the implementation especially the heavy ::nodejs::install are too complex and require a refactoring to split it into smaller components and the symlink mess should be cleaned up (using .profile.d scripts to link $PATH into linked directories)

The first idea was to use a tool like NVM or Nodenv to handle the issue with multiple versions in a better way, but both tools would introduce issues and similar tools would cause similar issues:

  • NVM: due to the approach of saving all the instances in a user's directory it seems to be a development tool rather than something for production. Furthermore it is unable to handle multiple versions in parallel which will be an impact for users who used multiple node versions with this module and ran them in parallel.
  • Nodenv: this module is able to handle multiple versions in parallel, but uses a hack like rbenv does called shims: those "executables" select the proper node version dependending on the working directory and redirect the console arguments to the actual executable. This might be great for people using multiple node versions, but at first we would be forced to rely on a foreign installer and as most of the people won't have the usecase of multiple versions (most of the issues aren't related to multi-versions) and I'm not a fan of forcing those people using some hacks to gain a feature which is used by a minority.

Further stuff to be refactored

  • bump the puppet version: 2.7 is still supported although it is quite ancient and not supported anymore. The least version should be 3.4.
  • remove legacy code: node 0.6 has still some custom source code as it isn't shipped with NPM, but it is old and not supported anymore: Consider dropping Node.js 0.6 support travis-ci/travis-ci#1785
  • simplify the installation: as described above it is not possible to switch the default instance easily and hiera can't be used for setting up multiple versions.

New approach

Multi-version support

For the reasons described above the code should be refactored, but the actual approach should be kept.

The ::nodejs::install should be declared as private and the internal process should be simplified.
The new code could look like this:

nodejs:
  default: v5.8.0  # default version
  instances:
    node-v5.8.0:
      version: v5.8.0
      target_dir: /usr/local/node
      make_install: false
  absent_versions: ['v0.12.2'] # it should be possible to remove versions

This should create the node-v5.8.0 and npm-v5.8.0.
As it is the default version, it should be symlinked into /usr/local/bin.

A note to the broken npm symlinks

Although all people who don't use the multi-version feature should not be forced to rely on pseudo-executables, the people who use this feature have to in order to be able to provide multi-version support of npm properly.

More features

The tickets #137 and #138 request a feature which basically means to support NPM configs (environment variables or NPM configs) and those should be supported using a generic approach:

Node environment variables

Add vars:

nodejs::environment_vars:
  ensure: present
  vars:
    NODE_PATH: /node/path
    NODE_ENV: production

And remove:

nodejs::environment_vars:
  ensure: absent
  vars: ['NODE_PATH']
Node configuration parameters:

Adding:

nodejs::npm_configuration:
  ensure: present
  vars:
    key: value

And removing:

nodejs::npm_configuration:
  ensure: absent
  vars: ['key']

Support

The changes won't be that hard as assumed before, therefore it should be kept for about 3 months after the 2.0-stable release.

@Ma27 Ma27 added this to the 2.0 milestone Mar 29, 2016
@Ma27
Copy link
Collaborator Author

Ma27 commented Mar 29, 2016

/cc @willdurand what do you think about that?

@Ma27 Ma27 self-assigned this Mar 29, 2016
@Ma27 Ma27 added the approved label May 27, 2016
@Ma27 Ma27 changed the title refactor the installer for 2.0 [RFC] refactor the installer for 2.0 May 27, 2016
@Ma27
Copy link
Collaborator Author

Ma27 commented May 27, 2016

as this RFC will be the approach how to implement the installer for 2.0, it can be closed now.
For further information about the approach please have a look at this: http://ma27.github.io/puppet/2016/05/05/next-puppet-nodejs-release.html

@Ma27 Ma27 closed this as completed May 27, 2016
@Ma27
Copy link
Collaborator Author

Ma27 commented Jan 18, 2017

see the 2.0 projectw hich contains all the necessary tasks...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

1 participant