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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

No way to build without --release? #463

Closed
anurbol opened this issue Nov 10, 2019 · 16 comments
Closed

No way to build without --release? #463

anurbol opened this issue Nov 10, 2019 · 16 comments

Comments

@anurbol
Copy link

anurbol commented Nov 10, 2019

Hi guys 馃憢 Is it me or it's only possible to neon build --release?
If I run just neon build then I get this message Neon only build with --release.... What is the reason of that?

There are some conditional attributes in my code that depend on debug_assertions, so those don't work because of this.

Response would be greatly appreciated, especially if soon.

@anurbol
Copy link
Author

anurbol commented Nov 10, 2019

Is this behavior just for Windows?

neon/src/lib.rs

Line 341 in 845425b

#[cfg(all(windows, not(neon_profile = "release")))]

Why is that, do anyone know? 馃槩

@kjvalencik
Copy link
Member

This is a limitation of the windows build process. If you build on other OS, debug builds work as expected.

If you need windows support, a workaround is to add the following to your Cargo.toml to enable debug assertions on release builds.

[profile.release]
debug-assertions = true

@anurbol
Copy link
Author

anurbol commented Nov 10, 2019

@kjvalencik Thanks so much for quick response. Does it have something to do with node-gyp and msvc? If yes, then I understand.

@anurbol anurbol closed this as completed Nov 10, 2019
@kjvalencik
Copy link
Member

kjvalencik commented Nov 10, 2019

To be honest, I'm not sure. It's been there a long time and I've never questioned it since I'm not a Windows user. I'll see if I can dig up an answer and get it documented.

@anurbol
Copy link
Author

anurbol commented Nov 10, 2019

Would be great. I am a newbie in Rust, but I'll probably try to test what happens if I remove that compile error line, and will write here.

@anurbol
Copy link
Author

anurbol commented Nov 10, 2019

Uhm.. It seems it compiles. I'll check if everything works from JS side.

@anurbol anurbol reopened this Nov 10, 2019
anurbol added a commit to anurbol/neon that referenced this issue Nov 10, 2019
@anurbol
Copy link
Author

anurbol commented Nov 10, 2019

@kjvalencik Everything works on windows. I removed the line from ~/.cargo/registry/.../neon-0.3.3
and everything builds just fine; also JS has no troubles requiring the built module. Made a pull request to comment those lines in the repo. Maybe the problem was fixed in node-gyp or somewhere, who knows. I hope you'll be able to dig up the old reason why this was done.

@anurbol
Copy link
Author

anurbol commented Nov 25, 2019

Turns out there is some problem with building without the --release flag. This message shows up when running a node.js addon module that was successfully compiled without the flag: Error: internal error in Neon module: attempt to subtract with overflow.

@kjvalencik
Copy link
Member

Interesting. That seems like it's catching an actual bug. Are there any more details when run with RUST_BACKTRACE=1?

@anurbol
Copy link
Author

anurbol commented Nov 26, 2019

@kjvalencik I am not sure I did it correctly: I ran
$Env:RUST_BACKTRACE=1; neon build and then ran the js app that requires the addon. Still the same error Error: internal error in Neon module: attempt to subtract with overflow with no backtrace. What am I doing wrong?

@usagi
Copy link
Contributor

usagi commented Mar 24, 2020

I have the same interest for the issue. Have you @anurbol , @kjvalencik any additional infos?


I think, the issue is not a critical than before because developers who includes Windows target can use WSL2 environment in very easily. And we can split a NEON marshlling workspace and a backend core business logics workspace then we avoid the problem very easy too. But, the situation is not beautiful and confusible for newbiews then I want to solve it if possible. 馃

@kjvalencik
Copy link
Member

@usagi Most likely it's being suppressed by the overridden panic hook. You can try enabling the default-panic-hook feature flag in neon.

@usagi
Copy link
Contributor

usagi commented Mar 25, 2020

This post is a progress logging about my first trial of debug build in Windows. It's works at least the first "hello". 馃憤

  1. Modify the local clone of the neon repos for this experiment ( C:\Users\usagi\tmp\z\WINDOWS_SPECIALIZED\neon )
    1. Remove the safety code specialized for Windows from src/lib.rs
#[cfg(all(windows, not(neon_profile = "release")))]
compile_error!("Neon only builds with --release. For tests, try `cargo test --release`.");
  1. Create a new neon app for this experiment ( C:\Users\usagi\tmp\z\WINDOWS_SPECIALIZED\aaa )
    1. neon new aaa
    2. Modify the dependencies to the path of (1) in the native/Cargo.toml
      1. neon = { path = "../../neon" } in the [dependencies]
      2. neon-build = { path = "../../neon/crates/neon-build" } in the [build-dependencies]
  2. Build the app with debug
    1. Neon build
      • ; It could not be completed, but works until before the last linking of the aaa.dll.
      • ; And then, it displays the command logging of the linking stage with an errors.
    2. Copy and paste the linking command line to a text editor in temporaly because it is sooooooo long 馃崫
      • ; link.exe (brabrabra...) is it.
    3. Modify the command line manually; Because it looks like a simple common trouble about a msvc toolchain's MT or MTd runtimes mainly and I think I can fix quickly with hand writing in temporaly.
      1. Add: "msvcrtd.lib"
      2. Add: "/NODEFAULTLIB:libcmtd.lib"
      3. Remove: "msvcrt.lib"
      4. Remove: "/DEF:C:\\Users\\(your-user-name)\\AppData\\Local\\Temp\\rustcsVVlTF\\lib.def"
    4. Run the modified linking command line use the x64 Native Tools Command Prompt for VS 2019
      • ; You don't worry about relative paths because the command line made with absolute paths.
      • ; If you could not paste all of the command line then you may remove a part of paths of .lib arguments because it included of /LIBPATH: argument, maybe.
      • ; It will be create the aaa.dll file in the native/target/x86_64-pc-windows-msvc/debug/deps directory.
    5. Copy or rename the extension from aaa.dll to aaa.node
    6. Test the aaa.node with Node.js
      • ; eg. in command line: node -e "console.log(require('./native/target/x86_64-pc-windows-msvc/debug/deps/aaa.node').hello())" in the project root directory of aaa project.
      • ; eg. in Node.js interactive shell:
PS C:\Users\usagi\tmp\z\WINDOWS_SPECIALIZED\aaa\native> node
Welcome to Node.js v13.5.0.
Type ".help" for more information.
> const aaa = require('./target/x86_64-pc-windows-msvc/debug/deps/aaa.node')
undefined
> aaa.hello()
'hello node'
>

eg. with VSCode + LLDB debugger IDE

launch.json:

{
  "version": "0.2.0",
  "configurations": [
    {
      "type": "lldb",
      "request": "launch",
      "name": "Debug: Backend",
      "program": "node",
      "args": [
        "-e",
        "console.log(require('./native/target/x86_64-pc-windows-msvc/debug/deps/aaa.node').hello())"
      ],
      "cwd": "${workspaceFolder}"
    }
  ]
}

And then:

image

The debugger works fine, at least in the super simple "hello". 馃憤

Maybe, this problem of the linking stage is not the core of the problem. The main reason of the feature disabling by is in a more internal somewhere. But, if the problem is only it then It may solve easy. I guessing, it will be solve if fix the usage about node-gyp, or at least write a specialized linking process to a ".cargo/config" for Windows in temporarily, maybe.


Related commit logs:

Hmm, the main reason is not written in these commit logging, but it may mean the reason is a very simple. 馃挕

@usagi
Copy link
Contributor

usagi commented Mar 25, 2020

And then, I found minimum workaround or fixing for the linker trouble.

Add the below code to the pub fn setup for Windows version in the neon/crates/neon-build/src/lib.rs file:

            if debug
            {
                println!("cargo:rustc-link-lib=msvcrtd");
            }

Then, neon build (and of course cargo build in the native directory too) will be works expectedly, and cli (=neon command) create native/artifacts.json and index.node files. 馃憤

Summary of "How to build a debug version native/index.node in Windows experimentally" is:

  1. Modify the NEON codes:
    1. Remove/Comment the safety codes from the neon/src/lib.rs in temporaly; The details is in the previous my post.
    2. Add the msvcrtd fixing; The detail is in above of in this post.
  2. Modify the app codes:
    1. native/Cargo.toml:
      1. [dependencies]: neon = to { path=(your-modified-neon-path) }
      2. [devDependencies]: neon-build = to { path=(your-modified-neon-path)/crates/neon-build }
  3. Then, you can build a debug version use neon build.

But, it is not the solution for this issue yet. The debug version binary built using this method is not tested enough. I'll test with a native code debugger for more codes based on the examples. And, if it has not a critical problems then I'll create a PR.

@usagi
Copy link
Contributor

usagi commented Mar 25, 2020

@anurbol Could you share your happened code about #463 (comment) ? Of course don't worry, any infos are so helpful for me now anything either a simple hello-world or a more complexed something.

@usagi
Copy link
Contributor

usagi commented Mar 26, 2020

I'd completed a testing for all of the examples exclude 17(electron app), 18(publishing modules),21(Bindgen) that reconfigured to use the modified neon. No errors caused from the debug build mode with my modified neon. And, it could be native code debugging with msvc-debugger and lldb. Of course, the setting of these examples and the neon was modified to use with default-features = false and features = [ "default-panic-hook", "legacy-runtime" ] just in case.

Thus, I'll PR it soon. 馃帀

@dherman dherman closed this as completed in b68f4c1 May 8, 2020
dherman added a commit that referenced this issue May 8, 2020
dherman added a commit that referenced this issue May 15, 2020
dherman added a commit that referenced this issue May 15, 2020
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

No branches or pull requests

3 participants