Complaints about the adoption of the autoformatter RFC #166

We have a process for this, it’s called the Final Comment Period. It’s easy: Go to RFC Announcements - NixOS Discourse and then set the notifications to “Watching first post”. (Honestly it should be the default IMO)

Also note that this entire thing has been three years in the making. There were ample of warning signs that this was coming.

11 Likes

To stop this discussion from further going around in circles, let me re-state the motivating problems:

  1. Formatting across the Nixpkgs code base is inconsistent and of varying quality.
  2. There are reviewers with opinions about formatting, which can get very nit-picky and annoying
  3. On the other hand, sometimes the format of a contribution is just bad, and new contributors somehow need to learn the formatting rules.

One may argue about the severity of 1 and 3, but there is an established community consensus that 2. is a sufficiently annoying problem that it needs addressing.

Doing nothing is not an option. You cannot be against automatic Nix formatters without proposing a viable alternative.

14 Likes

I’m not telling people they can’t use nixfmt. My position is that people should respect others choices around these matters.

Yes… yes I can? When the cure is worse than the disease, the “do nothing” option always remains on the table.

This is the thin end of the wedge, but there are already commits that have reformatted a whole subtree of nixpkgs and now act as a backport-barrier occupying half of git blame with a semantically meaningless commit. And before someone tells me I can get git blame to ignore certain commits:

  1. it doesn’t do anything to help github’s web ui
  2. who’s going to keep a list of such commits?
  3. git can’t necessarily track changes past certain formatting changes because line correspondence gets blurred.

Foolishly I thought that no matter to what degree people developed & refined nixfmt, that we still had enough trust and respect for people in nixpkgs to be able to make their own choices about their editing and tooling.

I was responding to the overall topic of satisfaction regarding the formatting. While there are things I like about the formatter, there are others I really dislike (such as what it does to lib.attrByPath when the path is a list with multiple elements), but it is what it is.

I don’t agree with your position, but I also think it’s unlikely we can bridge the gap between us, so I didn’t particularly want to get involved in this sub-conversation.

Commits added to .git-blame-ignore-revs will be ignored by git blame. GitHub will also ignore them. One can see that in the formatted stdenv I linked.

2 Likes

This conversation kind of derailed from its original topic. Moved it in a separate thread.

11 Likes

Okay, I’ll stop here. I didn’t volunteer hundreds of hours of my time over the last two years to help solve a really hard problem the best I could, just to argue with a single armchair naysayer on the basis of “but I don’t wanna”. If you disagree with the current direction the project is going, whatever alternative proposal needs to one-up current Nixfmt. I demand constructive proposals, lest you’ll shout into the void.

Cheers

20 Likes

Thanks @picnoir I didn’t even know you could do that.

This is genuinely nice to know… :thinking: I wonder if it would be possible to auto-detect these commits somehow to generate that file…

However it still can’t solve the line-correspondence problem that occurs when e.g. lines are re-ordered.

As I said, however you choose to spend your time is your business (and actually I’ve no doubt that as a result, nixfmt is probably as good as an autoformatter can get). The problem is when you start from the position that you can steamroller everybody and mandate things that affect other peoples work. You (plural) see it as a “really hard problem”, others see it as people who just can’t silence the voices in their head that tut when they see the wrong type of trailing comma. And in return we all have to deal with things that have an actual effect on managing actual (semantically meaningful) code, and it turns us all into people who “care about formatting”, either because we need guidance configuring our tools, because it breaks an auto-backport, because it does something surprising and weird that we then open a (guaranteed extensive) thread about, or because it adds one extra complexity to interactive rebasing (which is already a struggle for so many people). If you want to summarize all these as “but I don’t wanna” you’re free to do so. Enforcing autoformatting is not going to end any discussions about formatting - it’s just going to pull everyone into the debate.

On the other hand I probably shouldn’t complain so much - yesterday was a good day, and I only lost 45m of my work-day to fighting peoples’ various autoformatters.

If we want examples of situations where hundreds of hours have been spent and to which the current consensus is still more or less “do nothing”, take a look at the problem of FOD cache poisoning - a very real security threat to the whole of nixpkgs that I’ve almost certainly spent hundreds of hours on considering mitigations for. Yet there’s scant little community interest in pursuing this subject very far and to date little has been done about it (weirdly I’m not using the amount of work I’ve put into this to justify forcing through one of my mitigations like a mandatory CI check, maybe that’s a trick I should learn…), and in comparison I feel genuinely embarrassed and worried when I see the amount of will and effort that people are putting into comma-fiddling. I guess at least an attacker’s PR introducing a cuckoo FOD to the binary cache won’t have been using the wrong sort of quotation mark, so that’s one thing.

2 Likes

There is a simple opt-out for your own code, don’t use the formatter. There is no plan to have the nix tool automatically format all nix files it encounters. We do plan to have a “nix fmt” available to improve access to the relevant tooling and help make it ubiquitous. So the remaining question is about Nixpkgs conventions.

For Nixpkgs, the feedback has been positive regarding having a formatter available and applied by default. There will be adoption pains, of course. It seems the complaint is not the typical style bikeshedding, but about the policy of enforcing style and the aggressiveness of those checks. Part of the motivation to be so aggressive about the checks is to reduce the transition period. Many of the annoyances you are facing is dealing with rebases and commits with/without formatting applied. Yes, this is annoying, lets try to get past it and get to a point where most of the in-flight changes all have formatting applied, this should reduce the headache.

I would be okay with specific suggestions on how to have an opt-out mechanism. I want one myself. But also note that this means your areas of Nixpkgs will be in a transition period longer than other areas. As a Nixpkgs policy this has been extensively considered with a vast majority of comments saying roughly “each formatter is bad, but having a formatter is great”.

11 Likes

Choosing to use or not a format is an opinion and because of that is not something you can prove that is better than the other; opinions have to be respected.

But as everything that is shared, you cannot impose your desire over other ones (even your desire to have no rule). As politicians represent us, nixpkgs maintainers also represent us, and they try to drive the project to a one that meets the desires of the majority. The RFC was accepted for a reason, the majority of the community wants a formatter.

@ris If you feel that the core maintainers (the decision makers) does not represent the community desire, or simply you don’t want to adapt to a new rule, this is not a law-issue, you can create your own nixpkgs and adopt that part of the community that does agree with you or make it private for yourself. However, is a simple rule of style so important to worth this? I don’t think so. You should be focused on new packages and features implementations rather than, as you say, unimportant formatting issues. Simply create the feature as if the format does not exist, then execute the auto-formatter and whatever other rules the community wants.

If the formatter breaks your work (something that should not happen) then report the bug, the formatter will evolve until a point that does not break anything. Also, prefer working in small chunks to avoid losing a big amount of time if this happens.

3 Likes

For what it’s worth, I gave up on contributing because I got tired of fighting with the formatting checker. bisq2: init @ 2.1.0 by emmanuelrosa · Pull Request #318594 · NixOS/nixpkgs · GitHub

1 Like

Should we change the error message printed when there is a formatting failure? There shouldn’t be any fighting involved; it ought to be as straightforward as running the exact command(s) printed by the GitHub check, committing, and pushing.

4 Likes

We just improved the CI message to be less ambiguous. I agree it was confusing before, but please report these things so we can improve them, or even better, make suggestions for improvements yourself with a PR. I think I can speak on behalf of the entire formatting team in saying that we want the formatting to be as easy and pleasant as possible, we’re not sadists that enjoy others struggling, and we’re happy to receive contributions to improve the situation.

And you can ping the formatting team on GitHub using @NixOS/nix-formatting, or on Matrix in #nix-formatting:nixos.org, and we also have bi-weekly open meetings to move forward: nixfmt/team/README.md at 14be7e665024f1a8c31d748b22f5e215856d3479 · NixOS/nixfmt · GitHub.

14 Likes

Also just opened workflows/check-nix-format: Mention who to ping for trouble by infinisil · Pull Request #337118 · NixOS/nixpkgs · GitHub so that people know who to ping if they’re having trouble with the CI check. I hope that we can reach a state where nobody needs to, but I don’t think we’re there just yet :slight_smile:

6 Likes

That’s an insightful perspective @tomberek, thankyou - my only worry is how healthy it will be to (tacitly) encourage people considering different parts of nixpkgs being their “territory”. Perhaps it’s something that already exists though.

Also thanks to everyone who hasn’t risen to my grumpiness on this subject. I’m usually the one in other projects fighting mandated local tooling because they’re assuming everyone to be running some sort of horrific homebrew/pyenv/rbenv setup and it doesn’t work well with my nix-based setup, and my main personal take-away from it all (beyond PTSD) has been not to make assumptions about other peoples local environments, because they likely have other things going on…

I find homogeneity and uniformity obscene in all their forms, but regarding auto-formatting specifically I believe Git is so miserably bad at merging that adopting just any kind of automation for this is a no-brainer decision

EDIT: OTOH probably the main reason I don’t feel mad about the RFC is that I happened to come across before it was accepted and I had all the opportunity to object, so I’ve no reason to think that my autonomy was in any way violated

1 Like

Git doesn’t do this by default (but GitHub indeed does, as do most other git forges, and various git porcelains add the feature by default), you need to add extra configuration for that:

[blame]
        ignoreRevsFile = ".git-blame-ignore-revs"

This causes issues with repositories that don’t have such a file, though, so it’s probably best to add this to specific repositories rather than your global config. Or not to use raw git in the first place, it has lots of these sharp edges, as I’m sure we all know…

4 Likes