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

Refactor mac installer (Rebase of #776 against master) #2571

Closed
wants to merge 1 commit into from
Closed

Refactor mac installer (Rebase of #776 against master) #2571

wants to merge 1 commit into from

Conversation

fhemberger
Copy link
Contributor

Can you please check, if the Mac installer is built and working correctly?

@brendanashworth brendanashworth added install Issues and PRs related to the installers. macos Issues and PRs related to the macOS platform / OSX. labels Aug 27, 2015
@pmq20
Copy link
Contributor

pmq20 commented Aug 27, 2015

Given that tools/osx-pkg.pmdoc/index.xml.tmpl has been deleted, why Makefile still has a line [1] read cat tools/osx-pkg.pmdoc/index.xml.tmpl?

[1] https://github.com/fhemberger/node/blob/refactor-mac-installer-rebase/Makefile#L351

@fhemberger
Copy link
Contributor Author

Because I forgot to remove it. ;) Good catch, thanks!

@@ -323,7 +323,22 @@ release-only:
exit 1 ; \
fi

$(PKG): release-only
pre-pkg:
touch tools/osx-pkg/scripts/iojs-run-uninstall # empty file for uninstall step
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs to be changed to touch tools/osx-pkg/scripts/nodejs-run-uninstall

@remixz
Copy link
Contributor

remixz commented Aug 27, 2015

With the couple of changes I noted on the files, this builds! 🎉 This is great, cheers to you @fhemberger. I had tried to rebase it a little while ago, but I unfortunately didn't get far. I'm very glad you did. 😄

@fhemberger
Copy link
Contributor Author

@remixz Phew, I'm glad it worked out. Thanks for your help!

@Fishrock123
Copy link
Contributor

Linking: #776

@silverwind
Copy link
Contributor

Very minor issue with the current installer: The logo is not scaled properly and is telling me 'No' :)

screen shot 2015-09-08 at 5 40 17 pm

@fhemberger
Copy link
Contributor Author

For the sidebar, I think we should go with a colored version oft the hexagon, instead of scaling the logo down. What do you think?

@silverwind
Copy link
Contributor

👍 would make better use of the available space than the scaled logo.

@fhemberger
Copy link
Contributor Author

Can someone please confirm that the rebased version is building correctly?
Also updated the installer logo:

@fhemberger
Copy link
Contributor Author

@nodejs/build Can we please merge this? Various incarnations of this PR have been open for months now, and I'd like to get this done, so we can start with the actual translation work (same for the Windows installer).

@rvagg
Copy link
Member

rvagg commented Sep 21, 2015

yes, but no, needs changes on the build machines so requires some additional work and testing, non-trivial merge, I'm sorry that I'm currently holding this one up, I'll (a) be trying to get to this as soon as I can and (b) removing myself as a bottleneck as we get more of the build team access to the osx machines

@fhemberger
Copy link
Contributor Author

< insert (almost) monthly nag here > 😄

As v5 seems to be already waiting around the corner, it would be great if this could land as well.

@jasnell
Copy link
Member

jasnell commented Oct 14, 2015

Does this supersede / replace #776?

@silverwind
Copy link
Contributor

@fhemberger can you provide some build instructions for the installer, so I can test it?

@fhemberger
Copy link
Contributor Author

@jasnell Yes, it's the changes of #776 rebased against the current master.

@silverwind Install Packages, then build for OS X:

./configure
make
make pre-pkg
make install DESTDIR=out/dist-osx
/usr/local/bin/packagesbuild tools/osx-pkg/osx-pkg-out.pkgproj

@silverwind
Copy link
Contributor

Gave it a few installs/uninstalls and it looks to be working great. Only nag I have is that it says 'Node.js was installed' after uninstalling. Other than that, LGTM.

@rvagg rvagg added this to the 5.0.0 milestone Oct 16, 2015
@rvagg rvagg mentioned this pull request Oct 16, 2015
13 tasks
@evanlucas
Copy link
Contributor

The current packages are named node-vx.x.x.pkg. This changes that to nodejs-vx.x.x.pkg. I would think that could break some external tools maybe?

@silverwind
Copy link
Contributor

Should be the node scheme: https://nodejs.org/dist/v4.2.1/

@fhemberger
Copy link
Contributor Author

@evanlucas Good catch, fixed.

@@ -0,0 +1,19 @@
#!/bin/sh
DIR_PREFIX=/usr/local
Copy link
Contributor

Choose a reason for hiding this comment

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

what happens if it was installed to a custom location?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can't choose a custom location in the installer.

Sorry, the screenshot is German but you get the idea.

Copy link
Contributor

Choose a reason for hiding this comment

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

ah good point

@evanlucas
Copy link
Contributor

Do we have a retina image available for the sidebar? I wonder if packages even supports that. If so, we should probably add that.

This seems to work well. @rvagg if there is anything I can do to help on the build side of things to get this in for v5.0.0, feel free to let me know :] Overall, LGTM for whenever this can land.

@jbergstroem
Copy link
Member

@evanlucas mac installers surely must be able to render a svg, no?

jpwesselink added a commit to jpwesselink/node that referenced this pull request Sep 15, 2017
Creates macOS pkg installer by using `pkgbuild` and `productbuild`.
Removes previous npm installation before installing npm.
Packages carry correct version attributes.
Support for intl installer features, defaults to `en`.
Fancy formatted license.
Renamed `osx` references to `macOS`.
Optional installation of npm

PR-URL: nodejs#15179
Fixes: nodejs#15012
Refs: nodejs#5656
Refs: nodejs#2571
Refs: nodejs#7097
BridgeAR pushed a commit that referenced this pull request Sep 28, 2017
Creates macOS pkg installer by using `pkgbuild` and `productbuild`.
Removes previous npm installation before installing npm.
Packages carry correct version attributes.
Support for intl installer features, defaults to `en`.
Fancy formatted license.
Renamed `osx` references to `macOS`.
Optional installation of npm.

PR-URL: #15179
Fixes: #15012
Refs: #5656
Refs: #2571
Refs: #7097
Reviewed-By: Lance Ball <lball@redhat.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
MylesBorins pushed a commit that referenced this pull request Sep 29, 2017
Creates macOS pkg installer by using `pkgbuild` and `productbuild`.
Removes previous npm installation before installing npm.
Packages carry correct version attributes.
Support for intl installer features, defaults to `en`.
Fancy formatted license.
Renamed `osx` references to `macOS`.
Optional installation of npm.

PR-URL: #15179
Fixes: #15012
Refs: #5656
Refs: #2571
Refs: #7097
Reviewed-By: Lance Ball <lball@redhat.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
addaleax pushed a commit to addaleax/ayo that referenced this pull request Sep 30, 2017
Creates macOS pkg installer by using `pkgbuild` and `productbuild`.
Removes previous npm installation before installing npm.
Packages carry correct version attributes.
Support for intl installer features, defaults to `en`.
Fancy formatted license.
Renamed `osx` references to `macOS`.
Optional installation of npm.

PR-URL: nodejs/node#15179
Fixes: nodejs/node#15012
Refs: nodejs/node#5656
Refs: nodejs/node#2571
Refs: nodejs/node#7097
Reviewed-By: Lance Ball <lball@redhat.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
MylesBorins pushed a commit that referenced this pull request Oct 3, 2017
Creates macOS pkg installer by using `pkgbuild` and `productbuild`.
Removes previous npm installation before installing npm.
Packages carry correct version attributes.
Support for intl installer features, defaults to `en`.
Fancy formatted license.
Renamed `osx` references to `macOS`.
Optional installation of npm.

PR-URL: #15179
Fixes: #15012
Refs: #5656
Refs: #2571
Refs: #7097
Reviewed-By: Lance Ball <lball@redhat.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
MylesBorins pushed a commit that referenced this pull request Oct 3, 2017
Creates macOS pkg installer by using `pkgbuild` and `productbuild`.
Removes previous npm installation before installing npm.
Packages carry correct version attributes.
Support for intl installer features, defaults to `en`.
Fancy formatted license.
Renamed `osx` references to `macOS`.
Optional installation of npm.

PR-URL: #15179
Fixes: #15012
Refs: #5656
Refs: #2571
Refs: #7097
Reviewed-By: Lance Ball <lball@redhat.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 16, 2018
Creates macOS pkg installer by using `pkgbuild` and `productbuild`.
Removes previous npm installation before installing npm.
Packages carry correct version attributes.
Support for intl installer features, defaults to `en`.
Fancy formatted license.
Renamed `osx` references to `macOS`.
Optional installation of npm.

PR-URL: #15179
Fixes: #15012
Refs: #5656
Refs: #2571
Refs: #7097
Reviewed-By: Lance Ball <lball@redhat.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 17, 2018
Creates macOS pkg installer by using `pkgbuild` and `productbuild`.
Removes previous npm installation before installing npm.
Packages carry correct version attributes.
Support for intl installer features, defaults to `en`.
Fancy formatted license.
Renamed `osx` references to `macOS`.
Optional installation of npm.

PR-URL: #15179
Fixes: #15012
Refs: #5656
Refs: #2571
Refs: #7097
Reviewed-By: Lance Ball <lball@redhat.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
MylesBorins pushed a commit that referenced this pull request Feb 11, 2018
Creates macOS pkg installer by using `pkgbuild` and `productbuild`.
Removes previous npm installation before installing npm.
Packages carry correct version attributes.
Support for intl installer features, defaults to `en`.
Fancy formatted license.
Renamed `osx` references to `macOS`.
Optional installation of npm.

PR-URL: #15179
Fixes: #15012
Refs: #5656
Refs: #2571
Refs: #7097
Reviewed-By: Lance Ball <lball@redhat.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
MylesBorins pushed a commit that referenced this pull request Feb 12, 2018
Creates macOS pkg installer by using `pkgbuild` and `productbuild`.
Removes previous npm installation before installing npm.
Packages carry correct version attributes.
Support for intl installer features, defaults to `en`.
Fancy formatted license.
Renamed `osx` references to `macOS`.
Optional installation of npm.

PR-URL: #15179
Fixes: #15012
Refs: #5656
Refs: #2571
Refs: #7097
Reviewed-By: Lance Ball <lball@redhat.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
MylesBorins pushed a commit that referenced this pull request Feb 13, 2018
Creates macOS pkg installer by using `pkgbuild` and `productbuild`.
Removes previous npm installation before installing npm.
Packages carry correct version attributes.
Support for intl installer features, defaults to `en`.
Fancy formatted license.
Renamed `osx` references to `macOS`.
Optional installation of npm.

PR-URL: #15179
Fixes: #15012
Refs: #5656
Refs: #2571
Refs: #7097
Reviewed-By: Lance Ball <lball@redhat.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
install Issues and PRs related to the installers. macos Issues and PRs related to the macOS platform / OSX.
Projects
None yet
Development

Successfully merging this pull request may close these issues.