deploy-rs icon indicating copy to clipboard operation
deploy-rs copied to clipboard

Could deactivate rollback too much if called on failure to set profile?

Open nagisa opened this issue 2 years ago • 0 comments

So I've been reading a code a little, and found a piece of code that seemed suspect to me:

The first failure mode in activation is when nix-env --set fails, in which case we invoke deactivate:

https://github.com/serokell/deploy-rs/blob/83e0c78291cd08cb827ba0d553ad9158ae5a95c3/src/bin/activate.rs#L369-L385

In turn deactivate will invoke nix-env --rollback:

https://github.com/serokell/deploy-rs/blob/83e0c78291cd08cb827ba0d553ad9158ae5a95c3/src/bin/activate.rs#L120-L126

--rollback is specified to reset as active the generation that has a highest number lower than the current generation. However in this case it isn't clear to me if the current generation number increases in the first place as a result of nix-env --set failing. In that case wouldn't deactivate rollback 1 generation too many (deleting the perfectly fine “current” generation?)

Should this code be fetching the “current generation” before attempting to activate a new one, and explicitly threading it through into deactivate so that deactivate knows exactly which generation to reset to?

nagisa avatar Apr 16 '22 13:04 nagisa