Improve detection and handling of local contribs with local changes before an update

d.o. does not maintain checksums of every file .. Yes, and MD5 on the whole package is enough IMO.

personally, I would be more interested in detecting changes according to svn and asking user to resolve those before downloading new version.

this is one of the things i will be tackling in the Drupal Ports system : http://groups.drupal.org/node/21295

Essentially we need the meta-information available before we can do a lot of package installation things.

this is one of the things i will be tackling in the Drupal Ports system : http://groups.drupal.org/node/21295

Essentially we need the meta-information available before we can do a lot of package installationt hings.

Note that checking the tarball's MD5 may not work because you will need to regenerate the tarball to compare with the checksum, and tar is known to not always generate the same output for the same input (if timestamps changed, for example). You could do the reverse and extract the tarball then compare, but I think this would be a bit of a stretch: at this point, we should just use CVS (or SVN or whatever) instead.

I agree, adding a diff command is not that hard, if you require the project to be versioned.

doing it with randomly downloaded files is nigh on impossible, because you simply do not have the reliable meta-information to be able to do this.

rsvelko, This sounds like a bad feature... why not use the CVS handler in drush (so you are checking out tagged releases from cvs.drupal.org). Then let CVS handle merging for you.

This type of check if forgotten is annoying and makes people maintain a log of manual changes in a file

You absolutely need to maintain that log of manual changes. IMHO unless you have a rock-solid procedure for documenting and maintaining module changes, you really shouldn't be changing those files at all.

I think the ideal workflow here is similar to how apt-get update handles configuration updates. Basically it halts execution (unless you have a predefined option set). Then there is a menu that gives you the ability to (a) keep your version (b) show package maintainers version (c) show a diff and return to menu (d) attempt to merge the changes (perhaps open an editor to resolve the conflicts) and (f) drop to a shell to examine the situation further (etc).

Obviously it would be quite a lot of work to get all the way to this goal, however I think there are several things we can do quite easily to improve usability here.

The first is before we even start the update - if we have a CVS checkout it is easy to to a cvs diff and determine if there are customizations. If there are then we should offer (at least) additional warnings before continuing, and allow that project to be listed differently and (optionally) not updated. There is a separate issue to allow admin ignoring of packages, that could also use the same pattern - standard [issue nid]*.patch could also perhaps trigger this behavior.

If there are multiple files listed in the diff we should ask if they want to do either of the below options for all files, or (if there is a single file, or if the user requests) check each file individually (which offers the same options per file).
a) Leave as is and let CVS attempt to merge (default if we have version control) - it would be nice to give some indication if we expect conflicts or if it looks clean...not sure how to do this in CVS though.
b) Write the diff to a patch file, and revert to maintainers version (up -dPC) - to allow you to clobber the changes and merge them back manually - in many cases this is easier than resolving automatic merge conflicts. (default if we don't have version control).
c) Revert to maintainers version (up -dPC) - handy if you already have previous patches organized by issue nid you need to update from d.o and apply manually.
d) Show a diff (using a configurable diff viewer) and return here.
[Potentially other options here, but trying to keep it simple for now - eventually it would be great to allow interactive merges]

Note that theoretically it is possible to do similar things with wget, by downloading the current version (or checking an archive tucked away somewhere) and using vanilla diff to check for changes and patch to merge. However, it would be much more of a PITA!

With SVN, we can easily make 2 improvements that should make things more robust:
1) Before any update operation we should also put in a check for uncommitted changes (svn status). If this fails we should probably exit (or at least set an error and skip that update).
2) Before any update operation we should check that the sandbox is up to date - i.e. that there are not incoming changes that could cause conflicts (svn status -u). If step 1 passes it should be fairly safe to update and continue - but we should ask first!
3) Check the SVN commit response more intelligently, and give the user clearer feedback if it failed, or if there were conflicts etc.

We should also theoretically be able to use SVN to determine the list of files customized and offer choices as above, however it is quite a lot harder, because there is no easy way of telling which revision is the latest "clean" version of the package. Potentially we could use some tagging convention, or check for commit messages made from drush, but both of these seem pretty fallible to me.

As usual, Grugnog2 speaks the truth here. Sounds lovely.

I would like to include the simple SVN improvements (stat and stat -u preflight checks and the commit outcome check) in this release if possible.

I'm reluctantly removing the critical priority here. If a committer disagrees, feel free to put it back.

See also comments in duplicate issue #487584: drush update causes modules' external dependencies to be removed

subscribing

subscribing

The hacked module has a feature to automatically lock modified files prior to pm-updatecode. Shall we consider this close enough, and close this issue, or does someone want to keep working on it in drush core?

I'd like to hear Owen's input before closing this. He's the assignee.

subscribing

This issue was marked closed (won't fix) because Drush has moved to Github.

If this feature is still desired, you may copy it to our Github project. For best results, create a Pull Request that has been updated for the master branch. Post a link here to the PR, and please also change the status of this issue to closed (duplicate).

Please ask support questions on Drupal Answers.