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

src: rename node to io.js for Windows installer #291

Closed
wants to merge 1 commit into from

Conversation

rvagg
Copy link
Member

@rvagg rvagg commented Jan 11, 2015

  • rename various files
  • quote "NODE_VERSION_STRING" in node.rc to allow for complex version strings
  • rename and reword config for wix installer config

A special nightly of this can be found @ https://iojs.org/download/nightly/v1.0.0-nightly201501111b8f37bb4d/ with the .msi and 64-bit executable in the win-x64 directory.

Installer background image files are not included, I'm adding them in as a special build step for now, we'll figure out what to do about them later because of the "each version gets a new logo" thing we're apparently doing. I haven't changed the .ico file yet, just renamed for now.

Please review and test .msi installer @piscisaureus @seishun and other Windows users. I'd like to get this initial pass landed asap and we can iterate from there.

@rvagg
Copy link
Member Author

rvagg commented Jan 11, 2015

What it currently looks like:

screenshot from 2015-01-11 21 09 51

screenshot from 2015-01-11 21 10 24

@piscisaureus
Copy link
Contributor

each version gets a new logo

We are? Let's not.

@seishun
Copy link
Contributor

seishun commented Jan 11, 2015

  1. The license agreement says "Copyright Joyent, Inc. and other Node contributors.", but everywhere else it's changed to "Copyright io.js contributors". Is it supposed to be like that?
  2. It uninstalled Node.js. This would make sense if it created a node symlink for me, but it didn't.

Otherwise I don't see any other immediate problems.

@piscisaureus
Copy link
Contributor

@rvagg

  • The installer you've built works great.
  • However, some of the updated imagery seems missing in your PR
  • When I try to build the MSI myself I get the following error:
The system cannot find the batch label specified - getiojsversion
Building iojs-
D:\iojs\tools\msvs\msi\product.wxs(14): error CNDL0006: The Product/@Version at
tribute's value cannot be an empty string.  If you want the value to be null or
 empty, simply remove the entire attribute. [D:\iojs\tools\msvs\msi\iojsmsi.wix
proj]
D:\iojs\tools\msvs\msi\product.wxs(14): error CNDL0010: The Product/@Version at
tribute was not found; it is required. [D:\iojs\tools\msvs\msi\iojsmsi.wixproj]
  • Let's not go too wild with renaming files and strings. I do believe that in the near future node and iojs will be built from the same source, so spraying 'iojs' all over the place will just cause us pain down the road. In many cases renames could just be done in a neutral way, e.g. getnodeversion -> getversion as opposed to getiojsversion.

@piscisaureus
Copy link
Contributor

In reply to myself:

However, some of the updated imagery seems missing in your PR

Yes, Rod said as much :)

@rvagg
Copy link
Member Author

rvagg commented Jan 12, 2015

I'll roll back some of the renaming in this.

However, I believe at some point we need to have the confidence to say that this project has a life of its own, not tied to the process the JNAB is crawling through. The further io.js and the JNAB process evolve separately, the more incompatible they will likely become and reunification may be more difficult. Just consider how we have created an agile, just get things done process and culture here and witness the heavy-handed, over-bureaucratised process the JNAB is heading towards. Even with "Node" in a foundation, there's going to be a big culture and process barrier and my guess is that the momentum will be on the io.js side and all we really want here is the ability to call this thing "node".


<?define RegistryKeyPath = "SOFTWARE\Node.js" ?>
<?define RegistryKeyPath = "SOFTWARE\io.js" ?>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@piscisaureus any idea what the implications of changing the registry key path might be?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Third-party apps that are looking for node in the registry won't find it. I think that's relatively uncommon though, so this seems okay to do.

@rvagg
Copy link
Member Author

rvagg commented Jan 12, 2015

rolled back a bunch of changes so it's mainly just user-facing strings that have been changed now.

@rvagg rvagg force-pushed the win-installer-fix branch 3 times, most recently from c88ffb7 to 84fa1f8 Compare January 12, 2015 05:29
@rvagg
Copy link
Member Author

rvagg commented Jan 12, 2015

screenshot from 2015-01-12 17 37 37
screenshot from 2015-01-12 17 38 39
screenshot from 2015-01-12 17 41 29
screenshot from 2015-01-12 17 41 49

@hughsk took pitty on me and made some new designs. These are closer to the original node installer but still distinctive enough.

@rvagg
Copy link
Member Author

rvagg commented Jan 12, 2015

https://iojs.org/download/nightly/v1.0.0-nightly2015011284fa1f8c46/ has both 64 and 32-bit installers using this branch

@rvagg
Copy link
Member Author

rvagg commented Jan 12, 2015

This is good to go IMO, the nightlies look solid and I can manually build an installer from a clean source using vcbuild.bat release nosign msi x64 (not providing any pre-set environment variables for the version / tag or anything) and I get iojs-v1.0.0-x64.msi in the Release/ directory.

@piscisaureus please review.

@rvagg rvagg added this to the v1.0.0 milestone Jan 12, 2015
@rvagg rvagg self-assigned this Jan 12, 2015
@rvagg rvagg mentioned this pull request Jan 12, 2015
14 tasks
@piscisaureus
Copy link
Contributor

@rvagg

LGTM

I still get the same error (batch label getnodeversion not found). That is a know bug in cmd.exe, it's not the first time I ran into it. You can fix it by duplicating the getnodeversion label.

all we really want here is the ability to call this thing "node".

I think we're mostly on the same page about that. Notice how I said "I do believe that in the near future node and iojs will be built from the same source" - which is why we must not make it too hard for ourselves to release this code base under the name "node".

@rvagg
Copy link
Member Author

rvagg commented Jan 12, 2015

@piscisaureus as in, put this in at the top of the subroutine?

:getnodeversion
:getnodeversion

Do you know what versions of cmd this affects? I'm doing this mostly on a 2008 box and haven't seen the problem.

@rvagg
Copy link
Member Author

rvagg commented Jan 12, 2015

@piscisaureus just browsing around this issue on the web it seems a common comment about it is that it happens if batch files don't have proper CRLF line-endings. Could it be it works for me because I told git to do windows-checkout unix-checkin and you have a dont-molest-the-files setup? I did edit this file in Linux after testing the fix on Windows so if I unix2dos the file perhaps it'll be OK?

@rvagg
Copy link
Member Author

rvagg commented Jan 12, 2015

I've done a unix2dos on vcbuild.bat on this branch, perhaps you can try it out @piscisaureus

@seishun
Copy link
Contributor

seishun commented Jan 12, 2015

Normally one doesn't store CRLF endings in the repo, but instead sets eol=crlf on the file in .gitattributes to make git automatically convert line endings to CRLF on checkout (and back to LF on commit). See here.

@piscisaureus
Copy link
Contributor

Oh, I never realized that CRLF could be the cause of this problem. Silly me. I always use \n in batch files and it works most of the time...
The .gitattributes trick by @seishun seems the way to go then.

* quote "NODE_VERSION_STRING" in node.rc to allow for complex version
  strings
* change user-facing strings
@rvagg
Copy link
Member Author

rvagg commented Jan 12, 2015

thanks for the tip @seishun, reverted the line-endings and amended .gitattributes

rvagg added a commit that referenced this pull request Jan 12, 2015
* quote "NODE_VERSION_STRING" in node.rc to allow for complex version
  strings
* change user-facing strings
* make sure .bat files are crlf

PR-URL: #291
Reviewed-By: Bert Belder <bertbelder@gmail.com>
Reviewed-By: Nikolai Vavilov <vvnicholas@gmail.com>
@rvagg
Copy link
Member Author

rvagg commented Jan 12, 2015

landed in 43e4c90, thanks both!

@rvagg rvagg closed this Jan 12, 2015
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

3 participants