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

nushell: 0.5.0, add --locked flag to cargo install #46505

Closed
wants to merge 1 commit into from

Conversation

samford
Copy link
Member

@samford samford commented Nov 8, 2019

  • Have you followed the guidelines for contributing?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with 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 brew install <formula>)?

Rust 1.39.0 is now in Homebrew (#46465), so we can remove the use of nightly in this formula. I also bumped the version and added the --locked flag to cargo install (per #46025).

When I tried to run brew test nushell locally, it hung on my system (macOS Catalina, 10.15.1) and wouldn't complete. Outside of the test, simply running nu opens the shell as expected. Let's see what happens on the CI server.

Formula/nushell.rb Show resolved Hide resolved
@samford
Copy link
Member Author

samford commented Nov 8, 2019

As expected, the build passes but it's just brew test that's failing. I'll have to look into this later when I have time but if someone wants to tinker with the test in the interim time, feel free.

@samford
Copy link
Member Author

samford commented Nov 9, 2019

I discovered the source of this issue after some testing with different versions of Nushell. Apparently the behavior of Ctrl+D changed after Nushell v0.2.0. For example, if you open a nu shell and press Ctrl+D, it doesn't exit like Bash or Zsh do.

The test was written with v0.2.0 of Nushell, so it passes in that context but hangs with Nushell v0.3.0 to v0.5.0. Looking at issues in the Nushell repo, they may revert the change in a future version but this particular test won't pass until there's a release with the expected Ctrl+D behavior.

I think for the interim time we may just have to use something like assert_match version.to_s, shell_output("#{bin}/nu --version") and then return to the proper test in the future, whenever possible. What are everyone's thoughts on this?

@chenrui333
Copy link
Member

I think for the interim time we may just have to use something like assert_match version.to_s, shell_output("#{bin}/nu --version") and then return to the proper test in the future, whenever possible. What are everyone's thoughts on this?

I think it works for me. This is really good rust formula though, so I definitely want to get it out and worry about the quality later on. :)

@chenrui333
Copy link
Member

chenrui333 commented Nov 19, 2019

Let's keep an eye on the future releases.

@chenrui333
Copy link
Member

@BrewTestBot test this please

@chenrui333
Copy link
Member

https://github.com/nushell/nushell/issues/714#issuecomment-554787767

@samford Oh, looks like they just reverted the change yesterday.

@samford
Copy link
Member Author

samford commented Nov 19, 2019

I meant to mention that yesterday but you've already found it. The next release should be back in line with the Ctrl-D behavior from version 0.2.0 and the proper test can be reinstated then.

I installed Nushall using the --head flag to build from the Git repo (using some of the modifications in this PR) and the original test completed rather than hanging. At least in my testing (on macOS 10.15.1), the CTRL-D\n text wasn't present in the output, so the previous test may have to be updated to the following to pass in the future:

assert_equal "#{Dir.pwd}> 2\n#{Dir.pwd}> ", pipe_output("#{bin}/nu", 'echo \'{"foo":1, "bar":2}\' | from-json | get bar | echo $it')

It seems like Nushell's recent release cadence is around a month but the current version number in Nushell's Cargo.toml file is 0.5.1, so they may push out a patch release containing the change rather than wait until the next minor release. Either way, I'll get a notification when the next release comes and I can create another PR if no one gets to it first.

@samford samford deleted the nushell-0.5.0 branch November 19, 2019 14:14
@waldyrious
Copy link
Contributor

FYI:

❯ brew upgrade
Updating Homebrew...
==> Auto-updated Homebrew!
...
==> Upgrading 2 outdated packages:
nushell 0.2.0_1 -> 0.5.0
imagemagick 7.0.9-4 -> 7.0.9-5
==> Upgrading nushell
==> Downloading https://homebrew.bintray.com/bottles/nushell-0.5.0.mojave.bottle.tar.gz

curl: (22) The requested URL returned error: 401 Unauthorized
Error: Failed to download resource "nushell"
Download failed: https://homebrew.bintray.com/bottles/nushell-0.5.0.mojave.bottle.tar.gz
Warning: Bottle installation failed: building from source.
==> Downloading https://github.com/nushell/nushell/archive/0_5_0.tar.gz
==> Downloading from https://codeload.github.com/nushell/nushell/tar.gz/0_5_0
######################################################################## 100.0%
==> cargo install --locked --root /usr/local/Cellar/nushell/0.5.0 --path .
...

Is this expected? Perhaps it's just a matter of time until the bottle becomes available?

@chenrui333
Copy link
Member

@waldyrious Just fixed it, thanks for bringing it up!

$ brew install --verbose nushell
Updating Homebrew...
==> Auto-updated Homebrew!
Updated 2 taps (homebrew/cask and caskroom/cask).
No changes to formulae.

==> Downloading https://homebrew.bintray.com/bottles/nushell-0.5.0.catalina.bottle.tar.gz
/usr/bin/curl -q --globoff --show-error --user-agent Homebrew/2.1.16-87-gd65ad9f\ \(Macintosh\;\ Intel\ Mac\ OS\ X\ 10.15.1\)\ curl/7.64.1 --fail --location --remote-time --continue-at 0 --output /Users/ruichen/Library/Caches/Homebrew/downloads/8251e65b43145edfe99cc4349028b86795e1a50b4df8a46548215babeee2aa75--nushell-0.5.0.catalina.bottle.tar.gz.incomplete https://homebrew.bintray.com/bottles/nushell-0.5.0.catalina.bottle.tar.gz
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 11.5M  100 11.5M    0     0  2636k      0  0:00:04  0:00:04 --:--:-- 2636k
==> Verifying 8251e65b43145edfe99cc4349028b86795e1a50b4df8a46548215babeee2aa75--nushell-0.5.0.catalina.bottle.tar.gz checksum
==> Pouring nushell-0.5.0.catalina.bottle.tar.gz
tar xof /Users/ruichen/Library/Caches/Homebrew/downloads/8251e65b43145edfe99cc4349028b86795e1a50b4df8a46548215babeee2aa75--nushell-0.5.0.catalina.bottle.tar.gz -C /var/folders/cb/s5w4_0p50639mbdvqhnmf7wm0000gn/T/d20191119-34942-16jepw5
cp -pR /var/folders/cb/s5w4_0p50639mbdvqhnmf7wm0000gn/T/d20191119-34942-16jepw5/nushell/. /usr/local/Cellar/nushell
chmod -Rf +w /var/folders/cb/s5w4_0p50639mbdvqhnmf7wm0000gn/T/d20191119-34942-16jepw5
==> Finishing up

@waldyrious
Copy link
Contributor

Confirmed, it works now. Thanks for the quick fix!

@waldyrious waldyrious mentioned this pull request Nov 27, 2019
5 tasks
@lock lock bot added the outdated PR was locked due to age label Jan 2, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Jan 2, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants