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.

Sign up for GitHub

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

Jump to bottom

build: make --shared-[...]-path work on Windows #21530

Closed
wants to merge 1 commit into from
Closed

build: make --shared-[...]-path work on Windows #21530

wants to merge 1 commit into from

Conversation

nornagon
Copy link
Contributor

The -L<path> syntax isn't recognized by link.exe, and gyp doesn't
translate it properly. Without this, link.exe generates the following
warning and fails to link:

LINK : warning LNK4044: unrecognized option '/LC:/Users/nornagon/...'; ignored
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added the build Issues and PRs related to build files or the CI. label Jun 25, 2018
@nornagon
Copy link
Contributor Author

Hmm, actually this doesn't work properly because gyp automatically adds .lib to the end of everything in the libraries list.

@nornagon
build: make --shared-[...]-path work on Windows
e8263cb 
The `-L<path>` syntax isn't recognized by link.exe, and gyp doesn't
translate it properly. Without this, link.exe generates the following
warning and fails to link:

```
LINK : warning LNK4044: unrecognized option '/LC:/Users/nornagon/...'; ignored
```
@nornagon nornagon force-pushed the win-shared-libpath branch from c4975e5 to e8263cb Compare June 25, 2018 19:47
nornagon added a commit to electron/node that referenced this pull request Jun 25, 2018
@nornagon
chore: make --shared-[...]-path work on Windows
9e77bb7 
The `-L<path>` syntax isn't recognized by link.exe, and gyp doesn't
translate it properly. Without this, link.exe generates the following
warning and fails to link:

```
LINK : warning LNK4044: unrecognized option '/LC:/Users/nornagon/...'; ignored
```

See nodejs/node#21530
@nornagon nornagon mentioned this pull request Jun 25, 2018
Merged
MarshallOfSound pushed a commit to electron/node that referenced this pull request Jun 27, 2018
@nornagon @MarshallOfSound
chore: make --shared-[...]-path work on Windows (#41)
4879332 
The `-L<path>` syntax isn't recognized by link.exe, and gyp doesn't
translate it properly. Without this, link.exe generates the following
warning and fails to link:

```
LINK : warning LNK4044: unrecognized option '/LC:/Users/nornagon/...'; ignored
```

See nodejs/node#21530
@addaleax
Copy link
Member

@nornagon I can’t really tell, but what’s the status of this? 🙂

@nornagon
Copy link
Contributor Author

My comment about "this doesn't work properly" referred to an older version of this patch that github doesn't seem to show. The current version of the patch works fine and is being used in Electron's fork of node (see electron/node#41).

@addaleax
Copy link
Member

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

/cc @nodejs/platform-windows

@addaleax addaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jul 19, 2018
@nornagon
Copy link
Contributor Author

The test that failed in ci was test.parallel/test-http2-respond-with-file-connection-abort on OS X, which seems unrelated to this change.

@joaocgreis
Copy link
Member

OSX re-run: https://ci.nodejs.org/job/node-test-commit-osx/19968/

codebytere pushed a commit to electron/node that referenced this pull request Jul 30, 2018
@nornagon @codebytere
chore: make --shared-[...]-path work on Windows (#41)
8bb69f2 
The `-L<path>` syntax isn't recognized by link.exe, and gyp doesn't
translate it properly. Without this, link.exe generates the following
warning and fails to link:

```
LINK : warning LNK4044: unrecognized option '/LC:/Users/nornagon/...'; ignored
```

See nodejs/node#21530
@maclover7
Copy link
Contributor

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

jasnell pushed a commit that referenced this pull request Aug 12, 2018
@nornagon @jasnell
build: make --shared-[...]-path work on Windows
83ab3bf 
The `-L<path>` syntax isn't recognized by link.exe, and gyp
doesn't translate it properly. Without this, link.exe generates
the following warning and fails to link:

```
LINK : warning LNK4044: unrecognized option '/LC:/Users/nornagon/...'; ignored
```

PR-URL: #21530
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: João Reis <reis@janeasystems.com>
@jasnell
Copy link
Member

jasnell commented Aug 12, 2018

Landed in 83ab3bf

@jasnell jasnell closed this Aug 12, 2018
rvagg pushed a commit that referenced this pull request Aug 13, 2018
@nornagon @rvagg
build: make --shared-[...]-path work on Windows
01147b4 
The `-L<path>` syntax isn't recognized by link.exe, and gyp
doesn't translate it properly. Without this, link.exe generates
the following warning and fails to link:

```
LINK : warning LNK4044: unrecognized option '/LC:/Users/nornagon/...'; ignored
```

PR-URL: #21530
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: João Reis <reis@janeasystems.com>
@rvagg rvagg mentioned this pull request Aug 13, 2018
Merged
This was referenced Aug 16, 2018
Closed
Closed
Closed
Closed
Closed
Closed
Closed
This was referenced Aug 16, 2018
Closed
Closed
Closed
codebytere pushed a commit to electron/node that referenced this pull request Aug 29, 2018
@nornagon @codebytere
chore: make --shared-[...]-path work on Windows (#41)
b175a5a 
The `-L<path>` syntax isn't recognized by link.exe, and gyp doesn't
translate it properly. Without this, link.exe generates the following
warning and fails to link:

```
LINK : warning LNK4044: unrecognized option '/LC:/Users/nornagon/...'; ignored
```

See nodejs/node#21530
codebytere pushed a commit to electron/node that referenced this pull request Sep 1, 2018
@nornagon @codebytere
chore: make --shared-[...]-path work on Windows (#41)
19f4a34 
The `-L<path>` syntax isn't recognized by link.exe, and gyp doesn't
translate it properly. Without this, link.exe generates the following
warning and fails to link:

```
LINK : warning LNK4044: unrecognized option '/LC:/Users/nornagon/...'; ignored
```

See nodejs/node#21530
alexeykuzmin pushed a commit to electron/node that referenced this pull request Sep 5, 2018
@nornagon @alexeykuzmin
chore: make --shared-[...]-path work on Windows (#41)
7687764 
The `-L<path>` syntax isn't recognized by link.exe, and gyp doesn't
translate it properly. Without this, link.exe generates the following
warning and fails to link:

```
LINK : warning LNK4044: unrecognized option '/LC:/Users/nornagon/...'; ignored
```

See nodejs/node#21530
codebytere pushed a commit to electron/node that referenced this pull request Sep 5, 2018
@nornagon @codebytere
chore: make --shared-[...]-path work on Windows (#41)
7a21f28 
The `-L<path>` syntax isn't recognized by link.exe, and gyp doesn't
translate it properly. Without this, link.exe generates the following
warning and fails to link:

```
LINK : warning LNK4044: unrecognized option '/LC:/Users/nornagon/...'; ignored
```

See nodejs/node#21530
codebytere pushed a commit to electron/node that referenced this pull request Sep 13, 2018
@nornagon @codebytere
chore: make --shared-[...]-path work on Windows (#41)
ea403e1 
The `-L<path>` syntax isn't recognized by link.exe, and gyp doesn't
translate it properly. Without this, link.exe generates the following
warning and fails to link:

```
LINK : warning LNK4044: unrecognized option '/LC:/Users/nornagon/...'; ignored
```

See nodejs/node#21530
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@addaleax addaleax addaleax approved these changes

@jasnell jasnell jasnell approved these changes

@joaocgreis joaocgreis joaocgreis approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. build Issues and PRs related to build files or the CI.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@nornagon @addaleax @joaocgreis @maclover7 @jasnell @nodejs-github-bot