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

Add module 'versioning' #247

Merged
merged 1 commit into from Oct 17, 2020
Merged

Conversation

erikarvstedt
Copy link
Collaborator

@erikarvstedt erikarvstedt commented Oct 12, 2020

This PR adds option nix-bitcoin.configVersion to inform users about backwards-incompatible changes and to provide migration hints.

Example

A notification about the recent lightning-loop update could have been implemented like this:

{
  version = "0.0.18";
  condition = config.services.lightning-loop.enable;
  message = ''
    The lightning-loop data dir location was changed.
    To move it to the new location, run the following shell command on your nix-bitcoin node:
    sudo mv ${config.services.lnd.dataDir}/.loop ${config.services.lightning-loop.dataDir}
  '';
}

To see it in action, checkout this example branch and run test/run-tests.sh eval

Example output:

This version of nix-bitcoin contains the following changes
that are incompatible with your config (version 0.0.17):

- The lightning-loop data dir location was changed.
To move it to the new location, run the following shell command on your nix-bitcoin node:
sudo mv /var/lib/lnd/.loop /var/lib/lightning-loop
(This change was introduced in version 0.0.18)

- dummy
(This change was introduced in version 0.1)

After addressing the above changes, set nix-bitcoin.configVersion = "0.1";
in your nix-bitcoin configuration.

Details

Note that nix-bitcoin.configVersion in examples/configuration.nix only needs to
be updated after adding new breaking changes.

@erikarvstedt erikarvstedt force-pushed the versioning branch 2 times, most recently from 5851dee to 8b1b6fb Compare October 12, 2020 11:59
@nixbitcoin
Copy link
Member

Strong Concept ACK. I think this is pretty much the ideal fix for our release migration issues.

Could we also throw alerts on new features that may interest a user who's forked off the examples/configuration.nix?

@erikarvstedt
Copy link
Collaborator Author

home-manager implements news alerts like this. The date of the latest news that was read is stored inside $HOME.

In nix-bitcoin, the news items to be shown could be determined by configVersion, analogous to the breaking change messages.
We could certainly add this later on.

Copy link
Member

@jonasnick jonasnick left a comment

Choose a reason for hiding this comment

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

That's super helpful. ACK mod nit.


incompatibleChanges = optionals
(version != null && versionOlder lastChange)
(builtins.filter (change: versionOlder change && (change.condition or true)) changes);
Copy link
Member

Choose a reason for hiding this comment

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

what does this "or true" do?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It returns true if the change has no condition attribute.

Copy link
Member

Choose a reason for hiding this comment

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

I see. Hadn't noticed that construction before.

@jonasnick
Copy link
Member

Needs rebase

Copy link
Member

@jonasnick jonasnick left a comment

Choose a reason for hiding this comment

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

ACK d3ece59

@jonasnick jonasnick merged commit ee2a37d into fort-nix:master Oct 17, 2020
@erikarvstedt erikarvstedt deleted the versioning branch December 15, 2020 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants