Feature proposal: Bundler: Add warning to the lockfile contents warning not to hand-edit it

[zofrex]

I see beginners (and some non-beginners) make this mistake quite frequently of hand-editing the lockfile to make changes. I think a warning message in the lockfile itself to not do this could be extremely effective in pointing people in the right direction.

This might look something like this:

# Warning: Do not edit this file by hand! You should only update this file by running Bundler

GEM
  remote: https://rubygems.org/
  specs:
    actioncable (7.0.3.1)
      actionpack (= 7.0.3.1)
      activesupport (= 7.0.3.1)
      nio4r (~> 2.0)
      websocket-driver (>= 0.6.1)
... etc ...

[zofrex]

Based on the PoC the smoothest way to add this was as a new section in the lockfile, which would look something like this:

EDIT WARNING
  Do not edit this file by hand!
  You should only update this file by running Bundler
  e.g. `bundle install` or `bundle update`
  See https://bundler.io for more information test

GEM
  remote: https://rubygems.org/
  specs:
    actioncable (7.0.3.1)
      actionpack (= 7.0.3.1)
      activesupport (= 7.0.3.1)

Questions:

1. Are we happy with it being a regular lockfile section? (Constraints: The first line cannot start with whitespace and needs to be constant over time, i.e. probably shouldn't contain any part of the message as we may want to reword it later. Subsequent lines need to start with whitespace. By convention, but not a hard constraint, it would be all caps.)
2. What name should the section have? For simplicity we would ideally not change this again as it is machine-readable, so let's pick one that everyone is really happy with? I think ideally this needs to be fairly human readable (as it will be the first line of the warning) but also not something that might ever collide with any future additions (so just "WARNING" might not be such a great name, for example).
3. What should the warning say? This is less important to settle on than the header as this can be updated easily in the future, but it's still worth discussing what this should contain. Is it a hard "do not edit by hand ever" or is this a grey area, where it's ok sometimes? What are the best things to suggest instead, or resources to point people to?
4. Should this message be added to existing lockfiles, or only to newly-created lockfiles? @simi suggested only newly created ones, but I think there would be more value in adding it to existing lockfiles too: a lot of projects already have lockfiles and developers editing them by hand, it would be valuable to nudge them onto the right path.

[deivid-rodriguez]

I feel a bit hesitant to add this, because the section feels artificial and adds complexity, backwards compatibility concerns and that kind of thing.

I think a better course of action would be:

• Document more prominently that this is not recommended.
• Keep getting Bundler smarter. Many times we are able to recover from these bad edits by discarding the incorrect lockfile information. And when we are not, I think we can probably still detect that there has been a bad edit, and include a tip about it in the error message.

I think we should first come up with a list of "usual bad edits", how we currently handle them, and how we could improve handling them. And from the conclusions of that make up a decision.

[zofrex]

Thank you for your comments!

the section feels artificial

Agreed! It doesn't actually have to match the other sections in style, it could look like this for example:

# warning: do not edit this file by hand
  You should only update this file by running Bundler
  e.g. `bundle install` or `bundle update`
  See https://bundler.io for more information

GEM
  remote: https://rubygems.org/
  specs:
    actioncable (6.1.6)
      actionpack (= 6.1.6)

Or whatever else we want, really.

If the presentation is the serious blocker, I'd be open to suggestions that would be acceptable, ignoring the parsing concerns for the time being, and then try to work out how to implement them (could be gated behind v3 for example if it cannot be made backwards compatible).

adds complexity

This I'm not so certain about. Adding a new section fits into the existing parser and writer very easily, I wouldn't say it added a lot of complexity in my PoC.

backwards compatibility concerns and that kind of thing

What are the specific concerns about backwards compatibility? If I feed a lockfile with the warning to an older version of Bundler, it doesn't break, it just removes the warning. That's maybe not ideal, you might get some thrashing on the lockfile if different people on the same team are using different versions of Bundler (although with newer versions this is a lot less likely now), but doesn't actually break anything.

If the potential for thrashing is an issue even though we have version activation now, we could reduce thrashing by rolling out in stages: adding the code to Bundler to preserve the warning if present, and otherwise do nothing differently, and only later activate the code to add the warning to lockfiles, so there's a greater chance that everyone's Bundler will preserve it once it's added.

I think a better course of action would be:

• Document more prominently that this is not recommended.
• Keep getting Bundler smarter. Many times we are able to recover from these bad edits by discarding the incorrect lockfile information. And when we are not, I think we can probably still detect that there has been a bad edit, and include a tip about it in the error message.

I think we should first come up with a list of "usual bad edits", how we currently handle them, and how we could improve handling them. And from the conclusions of that make up a decision.

I think everything you've written here has enormous value and I agree completely that they are worth pursuing.

I don't think this approach has been or will be fully effective for stopping people editing the lockfile in general.

Over the years I've worked with Ruby I keep seeing people do this, often people who maybe only casually work in Ruby projects — maybe most of their team's projects are Go and they have one Ruby app, or maybe they don't do a lot of serious local development and largely rely on existing CI tooling for actually building/running the app. Whatever the reason, they've come in to Ruby via a route that didn't give them an introductory lesson on Bundler. That's person category 1.

Category 2 is users who do generally know how to use Bundler, but don't know how to fix certain problems using it, so they hand-edit the lockfile.

I think the people in category 2 are more likely to be reached by documentation, and also more likely to hit the "bad edit" detection and see any message that is put there, because they're trying to fix edge case problems and therefore more likely to make a bad edit.

The people in category 1, though, I often see just making very, very basic updates to lockfiles. For example, a minor version bump. To them, everything works, it isn't a bad edit, and the outcome of this is that they develop the opinion that Bundler (and Ruby dev) is a slightly clunky, awkward thing. Even when it does go wrong and it's a bad edit, their take away isn't "I'm doing this wrong", it's "this tool, that I already think is inefficient and clunky, also sometimes randomly explodes. What a bad tool!".

Those are really the people I'm trying to reach here. They're not going to see the docs, and they might not actually cause bad edits, but nothing they are encountering is telling them not to hand-edit these files (other than me, on Slack 😉).

There is one suggestion a friend made when I was discussing this with them earlier, which I want to relay because it would enable better detection of hand-edits: Bundler could add a checksum to the lockfiles, so it could detect if it was hand-edited and display a warning (it could then trash the lockfile and recreate it, or maybe allow you to then continue if you know what you're doing?)

Personally, I prefer the warning, just because it prevents the behaviour, rather than punishing for it later (potentially quite a few minutes later if someone is depending on a git push & CI run to actually run Bundler), but I think it's an interesting alternative!

[simi]

ℹ️ few examples of warning in other lockfiles

• yarn.lock

# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
# yarn lockfile v1

• Cargo.lock

# This file is automatically @generated by Cargo.
# It is not intended for manual editing.
version = 3

• composer.lock

{
    "_readme": [
        "This file locks the dependencies of your project to a known state",
        "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies",
        "This file is @generated automatically"
    ],

• gradle.lockfile

# This is a Gradle generated file for dependency locking.
# Manual edits can break the build and are not advised.
# This file is expected to be part of source control.

[deivid-rodriguez]

I agree the complexity is not that much from your PoC, still new code we need to maintain, but yeah, should be ok. Regarding backwards compatibility, that's probably too big of a term for what I'm talking about. I'm just referring to the fact that new versions of Bundler will introduce this extra diff noise, so we need to decide whether to apply this only to new lockfiles. And also that we need to make sure that old versions of Bundler can properly deal with this section, etc

I do recall a couple of issues where user had manually edit the lockfile. One recent, where the Gemfile was irresolvable in the first place, so the edit mostly changed nothing (adding a platform to the PLATFORMS section). Another one older where someone had edited the requirements of some gems and Bundler was printing a helpful warning. In general I think Bundler deals reasonably well with manual edits.

I guess I though it was common knowledge that lockfiles should not be edited. And maybe it is, but also probably because other tools are trying to spread the word about it, so that would be a reason to do this.

I think a checksum for edit detection detection is indeed a nice idea, and I'm not worried about "punishing after the fact", sometimes things stick more into your brain when learnt the hard way 😆. But it has the same issues I'm bringing here about complexity/"compatibility". Although it does not feel as artificial!

Anyways, I won't be blocking this, it's a nice practice many others are doing and I hope it's a good measure to educate people.