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

ytop: Add "ytop" to ytop's error message #101098

Merged
merged 1 commit into from Oct 20, 2020
Merged

ytop: Add "ytop" to ytop's error message #101098

merged 1 commit into from Oct 20, 2020

Conversation

@9999years
Copy link
Contributor

@9999years 9999years commented Oct 19, 2020

Right now, running nixos-rebuild gives an obscure error:

$ nixos-rebuild switch
building Nix...
building the system configuration...
error: Abandoned by upstream. Consider switching to bottom instead
(use '--show-trace' to show detailed location information)

(And --show-trace adds no useful information.)

The error message doesn't indicate that ytop is what's causing the problem.
By adding ytop to the error message, configurations that still reference ytop will be easier to debug and fix.

Right now, running `nixos-rebuild` gives an obscure error:
```
$ nixos-rebuild switch
building Nix...
building the system configuration...
error: Abandoned by upstream. Consider switching to bottom instead
(use '--show-trace' to show detailed location information)
```

(And `--show-trace` adds no useful information.)

The error message doesn't indicate that `ytop` is what's causing the problem.
By adding `ytop` to the error message, configurations that still reference
`ytop` will be easier to debug and fix.
cole-h
cole-h approved these changes Oct 20, 2020
Copy link
Member

@cole-h cole-h left a comment

Diff LGTM. Just ran into this when bumping nixos-unstable. Thanks!

@samueldr samueldr merged commit ae2cea4 into NixOS:master Oct 20, 2020
16 checks passed
@xaverdh
Copy link
Contributor

@xaverdh xaverdh commented Oct 20, 2020

Actually in that file there are some other occurrences like this, maybe we can fix them all?

@berbiche
Copy link
Member

@berbiche berbiche commented Oct 20, 2020

Sorry, this is a mistake on my end. I had assumed that aliasing a package to throw an error would also give the attribute that failed to evaluate.

@9999years
Copy link
Contributor Author

@9999years 9999years commented Oct 20, 2020

No worries, @berbiche — it's a reasonable assumption and the stack-trace feature really could use some improvement. I'll see if I can fix the others @xaverdh.

@xaverdh
Copy link
Contributor

@xaverdh xaverdh commented Oct 20, 2020

No worries, @berbiche — it's a reasonable assumption and the stack-trace feature really could use some improvement. I'll see if I can fix the others @xaverdh.

Something like #101174 ?

@xaverdh
Copy link
Contributor

@xaverdh xaverdh commented Oct 20, 2020

Still as this seems a common and easy enough mistake to make, we should probably think about some form of automation.

@berbiche
Copy link
Member

@berbiche berbiche commented Oct 20, 2020

Linking the RFC mentioned on IRC: NixOS/rfcs#33

9999years added a commit to 9999years/nixpkgs that referenced this issue Oct 20, 2020
aliases.nix: errors should include package names

In `pkgs/top-level/aliases.nix`, `throw` was used to make packages that
were removed error with a more useful message than "attribute 'foobar'
missing, at <location>".

However, if the error message doesn't include the package's attribute
name, it can be difficult to determine what caused it. For example,
here's what building a configuration that referenced `ytop` looked like
recently (see NixOS#101098):

```
$ nixos-rebuild switch
building Nix...
building the system configuration...
error: Abandoned by upstream. Consider switching to bottom instead
(use '--show-trace' to show detailed location information)
```

Therefore, we modify string values in `aliases.nix` to prefix `Attribute
foobar in <nixpkgs> has been removed` to the reason message. This makes
the removed reasons a bit shorter and provides a place to unilaterally
improve these error messages in the future, rather than with one-off
changes or large sets of manual fixes.
@9999years 9999years deleted the patch-1 branch Oct 20, 2020
@9999years
Copy link
Contributor Author

@9999years 9999years commented Oct 20, 2020

Opened #101210 to automatically add attr names, so this mistake can't be made manually any more.

$ nix repl
Welcome to Nix version 2.3.7. Type :? for help.

nix-repl> :l .
Added 12609 variables.

nix-repl> ag
«derivation /nix/store/maa4dn0d0jf96n0ir04lcw37l2x19lv6-silver-searcher-2.2.0.drv»

nix-repl> ytop
error: Attribute ytop in <nixpkgs> has been removed; abandoned by upstream. Consider switching to bottom instead

nix-repl> deepin
error: Attribute deepin in <nixpkgs> has been removed; it was a work in progress that has been canceled. See: https://github.com/NixOS/nixpkgs/issues/94870

06kellyjac
Copy link
Contributor

06kellyjac commented on e19a5b3 Oct 23, 2020

Thanks for this fix, the original error really threw me off for a bit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants