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

win: backport manifest for v0.10 #2838

Closed

Conversation

joaocgreis
Copy link
Member

This enables os.release() to report correct versions.

@rvagg I know it's late, but any chance of including in v0.10.41?

cc @nodejs/platform-windows

orangemocha and others added 2 commits September 12, 2015 21:01
This is a port of 03e9352 .

Original commit message:

  Adding a compatibility section to node.exe embedded manifest so that
  Node is declared explicitly compatible with Windows 8.1. Required so
  that os.release() can return the correct version on Windows 8.1.

  See http://msdn.microsoft.com/en-us/library/windows/desktop/ms724451(v=vs.85).aspx

  Reviewed-by: Trevor Norris <trev.norris@gmail.com>
This is a port of b0dd3bf .

Original commit message:

  Windows 10 wasn't listed in the executable manifest.
  This caused problems with trying to detect Windows 10
  via `os.release()`.

  PR-URL: nodejs#2332
  Reviewed-By: Roman Reiss <me@silverwind.io>
@joaocgreis joaocgreis added windows Issues and PRs related to the Windows platform. land-on-v0.10 labels Sep 12, 2015
@silverwind
Copy link
Contributor

LGTM, 0.12 also needs this.

@joaocgreis
Copy link
Member Author

@silverwind working on the port, along with some other commits! Thanks for the review

@joaocgreis
Copy link
Member Author

CI: https://ci.nodejs.org/job/node-test-pull-request/287/

Will wait the required 72 hours and then land, unless @rvagg says this can and needs to land faster for the release.

@rvagg
Copy link
Member

rvagg commented Sep 13, 2015

not too late, we still have work to do on the build side to even make this release possible so there's plenty of time

orangemocha added a commit that referenced this pull request Sep 16, 2015
This is a port of 03e9352 .

Original commit message:

  Adding a compatibility section to node.exe embedded manifest so that
  Node is declared explicitly compatible with Windows 8.1. Required so
  that os.release() can return the correct version on Windows 8.1.

  See http://msdn.microsoft.com/en-us/library/windows/desktop/ms724451(v=vs.85).aspx

  Reviewed-by: Trevor Norris <trev.norris@gmail.com>

PR-URL: #2838
Reviewed-By: silverwind - Roman Reiss <me@silverwind.io>
joaocgreis pushed a commit that referenced this pull request Sep 16, 2015
This is a port of b0dd3bf .

Original commit message:

  Windows 10 wasn't listed in the executable manifest.
  This caused problems with trying to detect Windows 10
  via `os.release()`.

  PR-URL: #2332
  Reviewed-By: Roman Reiss <me@silverwind.io>

PR-URL: #2838
Reviewed-By: silverwind - Roman Reiss <me@silverwind.io>
@joaocgreis
Copy link
Member Author

Landed in 8002192 and c88a0b2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
windows Issues and PRs related to the Windows platform.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants