-
Notifications
You must be signed in to change notification settings - Fork 66
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
chore: Remove self-update from default crate features #1139
base: main
Are you sure you want to change the base?
Conversation
Personally I would actually recommend taking the self updater out of the default feature list entirely. Folks can and should get their updates by whatever means they first used to get it (distro packages, |
Personally, I'm up for adapting default features in the way, that you can package with default-features instead of using another newly introduced feature. I'm myself not a very big fan of having self-updatable executables. So I understand where you are coming from. Let's wait for @aawsome opinion though, and if he agrees we can use the mentioned changes to the default features, instead. I can update our CI/CD workflows then. |
I fully agree with @simonsan |
Thanks for the review and being willing to re-consider the defaults. I'll update my changes with that in mind. |
The self-updater feature is problematic for many different distribution channels including distro packages, direct `cargo install`, various package managers and containers, etc. It will even goof up folks installing from source builds they made themselves. Either the binary does not have permission and should not be trying to monkey with the system installed binaries or the rest of the things besides the binary will end up out of sync ith packaging. Distros have their own update mechanisms that shouldn't be tampered with by every app that comes along. This means distro have to build with --no-default-features for apps that include self-updaters by default, but that also means we have to maintain a list of features we do want. This is not only tedious it is error-prone because there is a very good chance of new features getting overlooked when doing version bumps. Additionally many other install workflows should be updating using those workflows, not being tampered with internally this way. The binaries posted on releases that people might download and place manually are good candidates for enabling this feature.
I've replaced 5c1089d with 8be7b74 which just drops the self-updater from the default flag list. I had a look at CI but it isn't apparent to me what ends up where. None of the RPM or other package builds should have this feature enabled (as the meta data for the package would be out of sync even if somehow the binary got updated). On the other hand any binaries that the end use would download and place manually themselves are candidates for having this re-enabled. @simonsan Feel free to tag team on this PR or point me in the right direction or whatever... |
5c1089d
to
8be7b74
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This needs to wait a bit with merging though, as I need to update the CD first to build rustic-nightly and release with the corresponding added features. Will merge it when ready. |
The self-updater feature is problematic for distro package builders where the executable does not have permission and should not be trying to monkey with the system installed binaries. Distros have their own update mechanisms that shouldn't be tampered with by every app that comes along. This means distro have to build with --no-default-features for apps that include self-updaters by default, but that also means we have to maintain a list of features we do want. This is not only tedious it is error-prone because there is a very good chance of new features getting overlooked when doing version bumps.
Setting up a feature group like this makes it much easier for distros to build without the unwanted features without getting out of sync with upstream enhancements over time.