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

bat: 0.15.0 -> 0.15.1 #87622

Merged
merged 1 commit into from May 12, 2020
Merged

bat: 0.15.0 -> 0.15.1 #87622

merged 1 commit into from May 12, 2020

Conversation

lilyball
Copy link
Member

Motivation for this change

https://github.com/sharkdp/bat/releases/tag/v0.15.1

One of the tests fails 100% in macOS due to the way the filesystem behaves, so I've patched it out. sharkdp/bat#983 is the upstream issue to track this, so hopefully we can remove the patch in the next version.

Oddly, trying to build this with sandboxing failed on the cargo vendor stage where it couldn't update the index. I don't recall having this issue before, so I'm wondering if something changed on the nixpkgs side? Not directly related to bat though, and I assume it's specific to macOS sandboxing too or someone would have noticed this problem before.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@ofborg ofborg bot requested a review from dywedir May 11, 2020 23:57
@bhipple
Copy link
Contributor

bhipple commented May 12, 2020

trying to build this with sandboxing failed on the cargo vendor stage where it couldn't update the index.

I haven't seen this before, but I can say nothing has changed on the Rust/Cargo side that I'm aware of. The index update does use a lot of network traffic, though, so maybe your connection just blipped?

@lilyball
Copy link
Member Author

It gave some sort of SSL certificate error, as though it didn’t have access to the system certificate store (due to sandboxing) or the Nix certificate store. But I didn’t go digging, I just confirmed that it was reproducible until I turned off the sandbox.

@bhipple
Copy link
Contributor

bhipple commented May 12, 2020

Result of nixpkgs-review pr 87622 1

5 packages built:
- bat
- bat-extras.batgrep
- bat-extras.batman
- bat-extras.batwatch
- bat-extras.prettybat

bhipple
bhipple approved these changes May 12, 2020
View reviewed changes
Copy link
Contributor

@bhipple bhipple left a comment

Choose a reason for hiding this comment

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

Worked just fine on my NixOS machine with sandboxing. Actual application LGTM as well, but we should probably wait for the OfBorg Darwin build.

@lilyball
Copy link
Member Author

builder for '/nix/store/70cs9i20hy0nsp7q9q50d0wba3fr1s0a-llvm-9.0.1.drv' failed with exit code 1

@dywedir
Copy link
Member

dywedir commented May 12, 2020

Thanks!

@dywedir dywedir merged commit 4096396 into NixOS:master May 12, 2020
@lilyball lilyball deleted the bat branch May 12, 2020 20:11
@lilyball
Copy link
Member Author

FWIW I just confirmed the issue I'm getting in the macOS sandbox for bat also reproduces with ffsend. Something must have changed in nixpkgs related to the cargo vendor step, unrelated to bat.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ajs124 ajs124 ajs124 approved these changes

@bhipple bhipple bhipple approved these changes

@dywedir dywedir Awaiting requested review from dywedir

Assignees
No one assigned
Labels
10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 11.by: package-maintainer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@lilyball @bhipple @dywedir @ajs124