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

release builds: rename sunos to smartos #500

Closed
jbergstroem opened this issue Sep 21, 2016 · 35 comments
Closed

release builds: rename sunos to smartos #500

jbergstroem opened this issue Sep 21, 2016 · 35 comments

Comments

@jbergstroem
Copy link
Member

jbergstroem commented Sep 21, 2016

[note: this post has been updated since its creation]

I would like to suggest that we from 7.0 and onwards rename our sunos builds to smartos. This means that the tarballs will change from node-v7.0.0-sunos-x64.tar.xz to node-v7.0.0-smartos-x64.tar.xz.

Rationale

Naming our smartos builds for sunos is false advertising seeing how they are built on smartos and as a result doesn't work on many configurations of Solaris (toolchain, baselayout, ..).

Suggested plan of execution

  1. From Node.js v7.0.0 and forward we will create tarballs using smartos instead of sunos.

This only affects v7 and forward (meaning 0.10, 0.12, 4.x and 6.x branches remain the same); nightlies, test builds and so forth.

  1. We communicate this with packagers (nvm likely most important) and give them time to prepare.
  2. We reintroduce sunos should the build group ever get access to hardware/software to reliably build/test Node.js on.

/cc @misterdjules @geek

@geek
Copy link
Member

geek commented Sep 21, 2016

Thanks for raising this issue.

We absolutely should rename them. I am in favor of naming them SmartOS.

@misterdjules
Copy link
Contributor

What impact would this have on nvm and other version managers? For instance, whennvm runs on SmartOS, it downloads binaries whose names match the pattern *sunos*. If binaries are renamed, some versions of nvm running on SmartOS won't be able to download binaries.

It would be interesting to get @ljharb opinion on this.

@ljharb
Copy link
Member

ljharb commented Sep 21, 2016

Renaming the existing ones would absolutely break many existing nvm users.

What would be ideal is duplicating them to be named "smartos" on all existing instances, and then only using "smartos" moving forward (after i've updated nvm and bumped it in travis-ci, ofc).

@jbergstroem
Copy link
Member Author

jbergstroem commented Sep 21, 2016

I'm just tired of our false advertising -- it's not like the binary will work on sunos anyway. Lets focus on the tooling that assumes it's called sunos on smartos and solve those scenarios. Which tools/repos/packagers do we need to talk to?

  • nvm :)

@misterdjules
Copy link
Contributor

@jbergstroem @ljharb One problematic use case I had in mind is a user who has already installed nvm, and who would want to install new SmartOS releases that would be available only under the *smartos* name.

What would the failure look like? Is there a way with the current and older versions of nvm to display a human readable error message that would suggest these users to upgrade to a newer version of nvm that supports these new *smartos* names?

I'm not sure if that's worth the effort depending on the number of users and how closely they follow nvm's and node's changes, but I think it's worth it to think about it, if only to avoid users confusion and a number of issues in nvm's issues tracker.

@jbergstroem
Copy link
Member Author

@misterdjules it sounds like an nvm problem though (which doesn't mean I don't care, just that its out of scope for this group). I think we should identify tools/repos/packagers and mention we're renaming; set up a timeframe when we can allow a "both will work" and then kill sunos.

@ljharb
Copy link
Member

ljharb commented Sep 21, 2016

@jbergstroem that's pretty harsh - it's a problem for anyone who has bookmarked that URL. Cool URLs don't change, and invalidating any URL on the internet is a breaking change for somebody. Please don't minimize the damage that this could do.

@ljharb
Copy link
Member

ljharb commented Sep 21, 2016

@misterdjules there is no way to alter what current nvm users see - only what new ones see.

@jbergstroem
Copy link
Member Author

@ljharb but this would only be for future releases, right?

@jbergstroem
Copy link
Member Author

nodejs-v7.0.0-sunos.tar.xz would in time become nodejs-v7.0.0-smartos.tar.xz throughout the ecosystem.

@ljharb
Copy link
Member

ljharb commented Sep 21, 2016

In time it would, and if v7 was the first one to not have sunos, such that some nvm users wouldn't be able to download it (but could continue to download all the same sunos versions they already were), such that they needed to upgrade nvm - then that'd be fine!

The only breakage I'm concerned about is that existing sunos files must remain working forever.

@jbergstroem
Copy link
Member Author

@ljharb: existing files will work forever, this is just about new releases.

@ljharb
Copy link
Member

ljharb commented Sep 21, 2016

In that case, just give me a week's notice and I'll update nvm, and bump it on travis-ci, and I'm fully in favor.

@jbergstroem it would simplify my code a lot if you backfilled all existing sunos files with smartos ones - is that a possibility?

@jbergstroem
Copy link
Member Author

jbergstroem commented Sep 21, 2016

@ljharb: I'll get back to you on that, but seeing how this affects a lot of legacy stuff I think no one is inclined on touching it (0.x releases, iojs, etc). I wouldn't place my bets on it. This'll likely be one of those xz vs gz things.

@jbergstroem
Copy link
Member Author

@geek, @misterdjules, @ljharb what are your thoughts on doing this for 7.x and forward?

@ljharb
Copy link
Member

ljharb commented Sep 28, 2016

I'm on board, but would like as much concrete notice as possible (and an example index.tab) to try to ship the change beforehand :-)

@jbergstroem
Copy link
Member Author

@ljharb cool. We just need to start somewhere and a major makes more sense.

@misterdjules
Copy link
Contributor

@jbergstroem Can we summarize what the current plan is in the original comment of this issue so that we can make sure we're on the same page?

Also, my apologies for not being responsive today, I'll be offline most of the time.

@jbergstroem
Copy link
Member Author

jbergstroem commented Sep 28, 2016

@misterdjules sure. Give me a few minutes.

Edit: done.

@ljharb
Copy link
Member

ljharb commented Sep 28, 2016

👍 OP LGTM

@jbergstroem
Copy link
Member Author

/cc @nodejs/build @nodejs/release

@misterdjules
Copy link
Contributor

@jbergstroem Thank you for outlining the current plan in the original comment! The plan looks good to me.

@jbergstroem
Copy link
Member Author

jbergstroem commented Sep 29, 2016

aside:

  1. we should rename @nodejs/platform-solaris and the documentation around the same time we do this
  2. I've chosen to leave above team. Although I do a lot of tinkering/build on them, I just don't have the day-to-day experience to help out with any proper debugging. With @geek being more active (and @misterdjules great effort) my contribution is pretty irrelevant anyway :)

@misterdjules
Copy link
Contributor

@jbergstroem Where is the platform-solaris team mentioned in the documentation? I can't find any occurrence.

@jbergstroem
Copy link
Member Author

@misterdjules I'm probably hallucinating; had this faint recollection of a document that listed teams and when to cc them.

@Trott
Copy link
Member

Trott commented Sep 29, 2016

You are probably recalling https://github.com/nodejs/node/blob/master/doc/onboarding-extras.md but that team is not listed there. (PR to add it if you want.)

@jbergstroem
Copy link
Member Author

@Trott on spot as always -- thanks. Since we don't mention other platforms I think we'll be fine for now.

@jbergstroem
Copy link
Member Author

So, I'd like to proceed with this but I haven't heard much from the build group and/or release group. The next step would be writing logic in the smartos section of the release job, get some tests out.

@evanlucas
Copy link

Let's just not forget to update https://github.com/nodejs/build/blob/master/setup/www/tools/dist-indexer/transform-filename.js when this happens. +1 from me

@jbergstroem
Copy link
Member Author

@evanlucas just pushed in https://github.com/nodejs/nodejs-dist-indexer

(note: this doesn't mean it's decided, just that we're preparing for it)

@jbergstroem
Copy link
Member Author

Ok, one week to go. I would suggest this change in the build job under OSTYPE=solaris -- any objections? If not, I'll run a few test jobs

if [[ ${NODE_VERSION:0:1} -ge "7" ]]; then
 OSTYPE=smartos
fi

@jbergstroem
Copy link
Member Author

I'm going to implement this for the upcoming 7.0.0rc1 unless anyone has any objections.

@jbergstroem
Copy link
Member Author

Just did an attempt for rc1 which failed. I will have a look at this and hopefully make it for rc2.

@jbergstroem
Copy link
Member Author

with 7.0 being out and all I will attempt to get this in for next 7.x release. Sorry for not making it in time; been pretty busy with Jenkins headaches. If anyone else wants to chip in, feel free.

@maclover7
Copy link
Contributor

Closing for now as something that would be nice to have, but seems to not currently be within our means. If someone wants to tackle this, please feel free to reopen.

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

No branches or pull requests

7 participants