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

Make debug builds the default #437

Closed
fitzgen opened this issue Nov 6, 2018 · 8 comments
Closed

Make debug builds the default #437

fitzgen opened this issue Nov 6, 2018 · 8 comments
Labels
question Further information is requested REVISIT waiting on response

Comments

@fitzgen
Copy link
Member

fitzgen commented Nov 6, 2018

cargo does dev builds by default and we should match that behavior.

We should also match the terminology: dev instead of debug.

@alexcrichton
Copy link
Contributor

I think there's a strong point in favor of this to match what Cargo does, having as little deviation from Cargo as possible in wasm-pack makes it more familiar to Rust developers and also makes general information about Cargo applicable to wasm-pack as well.

I also think, though, that this is arguably a good point to diverge from Cargo. A fair number of users have popped up on our channels asking why code is so slow, and it often comes back to manual wasm-bindgen usage without --release when building in Cargo. Most users of wasm-pack are probably brand new to Rust (and maybe wasm too!) and having our best foot forward by default may not be a bad idea.

I think I could personally go either way on this!

@fitzgen
Copy link
Member Author

fitzgen commented Nov 6, 2018

Yeah me too honestly.

I do feel more stronly about matching terminology, though.

fitzgen added a commit to fitzgen/wasm-pack that referenced this issue Nov 6, 2018
@ashleygwilliams ashleygwilliams added the question Further information is requested label Nov 9, 2018
@ashleygwilliams
Copy link
Member

i personally think that diverging is the right choice here, but am open to being convinced otherwise. i'm glad that we switched to match terminology- do you feel like we can close this for now and revisit closer to 1.0?

@alexcrichton
Copy link
Contributor

I think it's fine to take care of more pressing issues and revisit this possibly after 1.0

@chinedufn
Copy link
Contributor

chinedufn commented Feb 3, 2019

i personally think that diverging is the right choice here, but am open to being convinced otherwise.

Migrated a project to wasm-pack a few days ago and I'm just now realizing that I've been using release builds the entire time.

Noticed certain things that used to work (mostly things behind cfg(debug_assertions) not work lately but didn't link it to using wasm-pack since I made a bunch of other changes before I noticed.

I'm very used to dev builds being the default so it caught be my surprise that release builds are the default.

In some ways I can understand that for people new to Rust and coming in through WASM --release might save them some performance confusion, but even so they'll be getting slower builds as they try to iterate (like silly me was.. can't believe I didn't notice.. ha!) before they realize that they can speed things up with a debug build.

So.. for me it makes sense to be consistent with the rest of the Rust toolchain, but I can of course see that the flood of "why is my code slow!" is a concern.


Potential Solution

  1. By default wasm-pack build prints a very loud very noticeable message letting you know that you are running a debug or release build (whatever is decided to be best)

  2. This message also lets you know that you can silence this message with some --quiet flag or something..

  3. No more performance confusion or inconsistency-with-cargo confusion (unless you just don't notice the loud message)

Any downsides / challenges here? Anything that I've overlooked? Mainly just don't want someone else to feel the same confusion that I just felt to the point where I was looking up if cfg(debug_assertions) still worked ha!

@chinedufn
Copy link
Contributor

Potential Solution 2

Don't default to any profile and make passing in --dev/release/profile required.

Upside being not relying on people reading stdout / downside being one extra thing that's required to get started.

@chinedufn
Copy link
Contributor

Sorry one other thing that comes to mind!

I just went to go to all of the places that I use wasm-pack test to set it to use a debug build, but it looks like wasm-pack test already defaults to debug builds.

So whether or not build/test should be consistent is yet another consideration here.

Ok I think I'm done... cheers!

@ibaryshnikov
Copy link
Member

+1 for making --dev builds by default. Release builds confuse me too. @chinedufn offered a great solution with logging.
Here are the current messages

cargo build:

Finished dev [unoptimized + debuginfo]

cargo build --release:

Finished release [optimized]

wasm-pack build:

.
.
Finished release [optimized]
.
.
📦   Your wasm pkg is ready to publish

wasm-pack build --debug:

.
.
Finished dev [unoptimized + debuginfo]
.
.
📦   Your wasm pkg is ready to publish

Why is the last message pkg is ready to publish when it's unoptimized + debuginfo?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested REVISIT waiting on response
Projects
None yet
Development

No branches or pull requests

5 participants