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

deno: downgrade to 1.43.1 #171382

Merged
merged 1 commit into from May 10, 2024
Merged

deno: downgrade to 1.43.1 #171382

merged 1 commit into from May 10, 2024

Conversation

chenrui333
Copy link
Member

@chenrui333 chenrui333 commented May 10, 2024 •
edited

  • Have you followed the guidelines for contributing?
  • Have you ensured that your commits follow the commit style guide?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with HOMEBREW_NO_INSTALL_FROM_API=1 brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
  • Does your build pass brew audit --strict <formula> (after doing HOMEBREW_NO_INSTALL_FROM_API=1 brew install --build-from-source <formula>)? If this is a new formula, does it pass brew audit --new <formula>?

closes #171317

@chenrui333 chenrui333 added CI-syntax-only Change only affects brew syntax, not the install. Only run syntax CI. CI-version-downgrade Pass --skip-stable-version-audit to brew test-bot. labels May 10, 2024
@github-actions GitHub Actions

This comment was marked as resolved.

Sign in to view
@github-actions github-actions bot added rust Rust use is a significant feature of the PR or issue long build Needs CI-long-timeout labels May 10, 2024
@chenrui333 chenrui333 marked this pull request as draft May 10, 2024 15:51
@chenrui333
deno: downgrade to 1.43.1
81767d8 
Signed-off-by: Rui Chen <rui@chenrui.dev>

deno: update test

Signed-off-by: Rui Chen <rui@chenrui.dev>
@chenrui333 chenrui333 marked this pull request as ready for review May 10, 2024 15:59
@jerome-benoit
Copy link

If the root cause is the rust toolchain version 1.78 usage to build Deno, I'm afraid the version downgrade will not be enough.

@chenrui333
Copy link
Member Author

seems working fine in my local?

$ clang --version
Homebrew clang version 18.1.5
Target: arm64-apple-darwin23.4.0
Thread model: posix
InstalledDir: /opt/homebrew/Cellar/llvm/18.1.5/bin


$ rustc --version
rustc 1.78.0 (9b00956e5 2024-04-29) (Homebrew)

$ brew test -d -v deno
/opt/homebrew/Library/Homebrew/vendor/bundle/ruby/3.1.0/bin/bundle clean
/opt/homebrew/Library/Homebrew/brew.rb (Formulary::FromNameLoader): loading deno
/opt/homebrew/Library/Homebrew/brew.rb (Formulary::FromTapLoader): loading homebrew/core/deno
/opt/homebrew/Library/Homebrew/brew.rb (Formulary::FromNameLoader): loading cmake
/opt/homebrew/Library/Homebrew/brew.rb (Formulary::FromNameLoader): loading llvm
/opt/homebrew/Library/Homebrew/brew.rb (Formulary::FromNameLoader): loading ninja
/opt/homebrew/Library/Homebrew/brew.rb (Formulary::FromNameLoader): loading protobuf
/opt/homebrew/Library/Homebrew/brew.rb (Formulary::FromNameLoader): loading rust
/opt/homebrew/Library/Homebrew/brew.rb (Formulary::FromNameLoader): loading sqlite
==> Testing deno
/opt/homebrew/Library/Homebrew/test.rb (Formulary::FromPathLoader): loading /opt/homebrew/Library/Taps/homebrew/homebrew-core/Formula/d/deno.rb
==> /opt/homebrew/Cellar/deno/1.43.1/bin/deno run hello.ts
==> /opt/homebrew/Cellar/deno/1.43.1/bin/deno run https://deno.land/std@0.100.0/examples/welcome.ts
/opt/homebrew/Library/Homebrew/test.rb (Formulary::FromNameLoader): loading sqlite

@jerome-benoit
Copy link

seems working fine in my local?

Does the resulting binary pass the repro: deno eval "fetch('http://8.8.8.8')"?

If yes, then the root cause is narrowed down to rust 1.78 and Deno 1.43.2 code base build.

@chenrui333
Copy link
Member Author

chenrui333 commented May 10, 2024 •
edited

seems working fine in my local?

Does the resulting binary pass the repro: deno eval "fetch('http://8.8.8.8')"?

If yes, then the root cause is narrowed down to rust 1.78 and Deno 1.43.2 code base build.

yeah, that works with 1.43.1, not 1.43.2 (left a comment in the issue thread as well)

env: llvm 18.1.5, rust 1.78.0

@carlocab
Copy link
Member

seems working fine in my local?

Does the resulting binary pass the repro: deno eval "fetch('http://8.8.8.8')"?
If yes, then the root cause is narrowed down to rust 1.78 and Deno 1.43.2 code base build.

yeah, that works with 1.43.1, not 1.43.2 (left a comment in the issue thread as well)

env: llvm 18.1.5, rust 1.78.0

I don't think @chenrui333 rebuilt deno from source with this, so new Rust/LLVM is still a candidate explanation for the failure. I'll give it a try.

@chenrui333
Copy link
Member Author

I did not perform source build (the 1.43.1 was built with llvm 17 as well)

we can potentially try this patch for source build with llvm 18, denoland/deno_core#735

carlocab
carlocab approved these changes May 10, 2024
View reviewed changes
Copy link
Member

@carlocab carlocab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok with merging this for now.

Bo98
Bo98 approved these changes May 10, 2024
View reviewed changes
Copy link
Member

@Bo98 Bo98 left a comment •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK with this given 1.43.3 will fix this.

Test seems a bit weird and I feel like it could be slimmed down but better than nothing.

@chenrui333 chenrui333 added this pull request to the merge queue May 10, 2024
@chenrui333
Copy link
Member Author

thanks, merging!

@chenrui333
Copy link
Member Author

chenrui333 commented May 10, 2024 •
edited

Test seems a bit weird and I feel like it could be slimmed down but better than nothing.

that just trying to reproduce the issue for the fresh-project checkout. (deno 1.43.2 just did a partial init, while deno 1.43.1 did the full project init)

Merged via the queue into master with commit 0151465 May 10, 2024
18 checks passed
@chenrui333 chenrui333 deleted the deno-downgrade branch May 10, 2024 16:51
@carlocab carlocab mentioned this pull request May 11, 2024
Closed
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Bo98 Bo98 Bo98 approved these changes

@carlocab carlocab carlocab approved these changes

@SMillerDev SMillerDev Awaiting requested review from SMillerDev

Assignees
No one assigned
Labels
CI-syntax-only Change only affects brew syntax, not the install. Only run syntax CI. CI-version-downgrade Pass --skip-stable-version-audit to brew test-bot. long build Needs CI-long-timeout rust Rust use is a significant feature of the PR or issue
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Deno 1.43.2 homebrew build segmentation fault
4 participants
@chenrui333 @jerome-benoit @carlocab @Bo98