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

[Tracking] Remove all uses of lib.mdDoc #300735

Closed
philiptaron opened this issue Apr 1, 2024 · 11 comments
Closed

[Tracking] Remove all uses of lib.mdDoc #300735

philiptaron opened this issue Apr 1, 2024 · 11 comments
Labels
5. scope: tracking Long-lived issue tracking long-term fixes or multiple sub-problems 6.topic: architecture Relating to code and API architecture of Nixpkgs 6.topic: documentation Meta-discussion about documentation and its workflow

Comments

@philiptaron
Copy link
Contributor

philiptaron commented Apr 1, 2024

Since #237557 was merged, lib.mdDoc has been a no-op. Let's remove the places that use it.

$ rg -w '(lib\.)?mdDoc' -o --no-filename | sort | uniq -c
  12356 lib.mdDoc
   1242 mdDoc
@philiptaron philiptaron added 6.topic: documentation Meta-discussion about documentation and its workflow 6.topic: architecture Relating to code and API architecture of Nixpkgs labels Apr 1, 2024
@fricklerhandwerk fricklerhandwerk changed the title Remove all uses of lib.mdDoc [Tracking] Remove all uses of lib.mdDoc Apr 9, 2024
@hojerst
Copy link
Contributor

hojerst commented Apr 15, 2024

as this seems to break the current home-manager project - wouldn't it be reasonable to wait a bit longer / issue a warning when this is used instead of removing it alltogether?

@eclairevoyant
Copy link
Contributor

wouldn't it be reasonable to wait a bit longer / issue a warning

There is a warning, unfortunately it's broken.

nix-repl> lib.options.mdDoc "test"
trace: warning: lib.mdDoc was removed from nixpkgs. Option descriptions are now in Markdown by default, you can remove any remaining uses of it.
"test"

nix-repl> lib.mdDoc "test"
error: attribute 'mdDoc' missing
       at «string»:1:1:
            1| lib.mdDoc "test"
             | ^

@philiptaron
Copy link
Contributor Author

Closed by #303841 (thanks @stuebinm!)

@stuebinm
Copy link
Contributor

ah sorry for the breakage everyone — I added that warning assuming it would be fine, but missed that mdDoc was only exposed via an inherit into lib.mdDoc (which of course got automatically removed along with all the other inherits).

@eclairevoyant
Copy link
Contributor

No worries, it'll be fixed soon enough. Thanks for the original PR; maybe this will get HM to remove the usages sooner too ;)

@nixos-discourse
Copy link

This issue has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/nix-darwin-is-broken/43497/2

@lorenzleutgeb
Copy link
Member

lorenzleutgeb commented Apr 22, 2024

Over at ngi-nix/ngipkgs we're running a CI job that evaluates with NIX_ABORT_ON_WARN=true. (We do this to catch option deprecations. I have seen many times more warnings for the user than for the developer, like lib.mdDoc deprecation.) Currently, lib.mdDoc still seems to exist but print a warning (see #304277) which consequently makes our builds fail (https://github.com/ngi-nix/ngipkgs/actions/runs/8780863139/job/24091654727).

Is there any chance the timeline for removing lib.mdDoc can be (significantly) accelerated? IMO removing it in Nixpkgs and telling downstream to fix it (they may also lib = lib // { mdDoc = x: x; } locally as a very quick workaround) is better than dragging on warnings for months.

Don't worry, if you respond with "no, we're clear on leaving it in until 24.11" then I'll not discuss any further and change the CI job, but it's not entirely clear to me that this is so fixed at this point...

@eclairevoyant
Copy link
Contributor

Giving a reasonable deprecation period is how we should handle anything removed from lib. In this case it was removed from hm and nix-darwin fairly quickly, but apparently we still have a lot more consumers using this function. At the very least they should know why they are getting this error for now.

Currently, lib.mdDoc still seems to exist but print a warning (see #304277) which consequently makes our builds fail

Besides, I do find it a little strange that simply defining the function in nixpkgs without using it would fail the CI.

@stuebinm
Copy link
Contributor

@lorenzleutgeb you should only get this warning if the lib.mdDoc function is actually used somewhere within the code you're evaluating. Unfortunately, due to how builtins.trace works, there is no easy way to find out where that use site is, which can indeed be annoying if it's not in your own code but somewhere in your dependencies (indeed the definition of lib.warn in lib/trivial.nix has a TODO item attached to it that essentially says "figure out a way to show users where the warning came from").

but from quickly looking at your repo, a reasonable bet is that you're getting it since dream2nix still has some uses of mdDoc in it, and you depend on it.

@lorenzleutgeb
Copy link
Member

lorenzleutgeb commented Apr 22, 2024

@stuebinm The offending option is actually printed: https://github.com/ngi-nix/ngipkgs/actions/runs/8780863139/job/24091654727#step:4:108 It seems that this occurrence of lib.mdDoc has been removed in the meantime, so I'll try updating the nixpkgs input of that Flake. We evaluate all of NixOS, i.e. all default modules of it, since we're generating option documentation, so it's not really relevant to trace down single occurrences. If it's somewhere in NixOS, it will fail our build.

@eclairevoyant
Copy link
Contributor

seems like it was fixed in #305379, so you'd need a commit after d9ad2e5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5. scope: tracking Long-lived issue tracking long-term fixes or multiple sub-problems 6.topic: architecture Relating to code and API architecture of Nixpkgs 6.topic: documentation Meta-discussion about documentation and its workflow
Projects
None yet
Development

No branches or pull requests

7 participants