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

Fix buffer overrun in cargo metadata command #4673

Closed
wants to merge 1 commit into from
Closed

Fix buffer overrun in cargo metadata command #4673

wants to merge 1 commit into from

Conversation

jamwaffles
Copy link

@jamwaffles jamwaffles commented May 29, 2020

↪️ Pull Request

Adds the --no-deps argument to reduce output length. A quick local test
cuts the output from ~1MB to ~10KB so with a default buffer of 1024*1024
bytes this should open up a lot more overhead.

Closes #1573

💻 Examples

Running parcel build or parcel path/to/index.html now no longer fails with src/lib.rs: stdout maxBuffer length exceeded

🚨 Test instructions

I believe this issue only occurs with larger projects. I have a cargo workspace containing an API server with a bunch of dependencies which I believe causes a huge amount of output. Testing this PR in that workspace with the added --no-deps flag makes it work.

I guess the test instructions would be to create/find a project with a bunch of dependencies and check the NPM version of parcel against an yarn link/npm link of this PR.

✔️ PR Todo

  • Added/updated unit tests for this change
  • Filled out test instructions (In case there aren't any unit tests)
  • Included links to related issues/PRs

Adds the `--no-deps` argument to reduce the output. A quick local test
cuts the output from ~1MB to ~10KB so with a default buffer of 1024*1024
bytes this should open up a lot more overhead.

Closes #1573
@height
Copy link

height bot commented May 29, 2020

Link Height tasks by mentioning a task ID in the pull request title or description, commit messages, or comments.

You can also use "Close T-X" to automatically close a task when the pull request is merged.

@jamwaffles
Copy link
Author

Uhh did I break the builds?

@DeMoorJasper
Copy link
Member

@jamwaffles we no longer use the master branch as that contains Parcel v1 and we're no longer actively working on that, besides releasing critical bugfixes if required

@jamwaffles
Copy link
Author

Ah, I see. If I retarget this branch to v2 is this something that could be merged into that?

@DeMoorJasper
Copy link
Member

We don't have rust support in v2 yet, so you'd also have to implement that first for it to get merged unfortunately.

@jamwaffles
Copy link
Author

Ok, thanks for explaining. That's a little beyond my Parcel expertise so I'll leave it to smarter people. I'll close this PR as it's putting the cart before the horse atm.

@jamwaffles jamwaffles closed this May 29, 2020
@jamwaffles jamwaffles deleted the fix-cargo-metadata branch May 29, 2020 12:01
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

Successfully merging this pull request may close these issues.

2 participants