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

Provide a way to not delete files or folders during pm-update drupal #614

Closed
kgaut opened this issue Apr 28, 2014 · 25 comments
Closed

Provide a way to not delete files or folders during pm-update drupal #614

kgaut opened this issue Apr 28, 2014 · 25 comments

Comments

@kgaut
Copy link

kgaut commented Apr 28, 2014

When running

  drush pm-update drupal 

All the files and folder within the rootpath are deleted, drupal's, but also, for example IDE's settings.

It would be nice to provide a way to not delete these files / folders, maybe by a drush.ignore file or by an argument to pm-update command.

What do you think about this idea? Do you have any suggestion ?

@kgaut kgaut changed the title Provide a way to not delete files or folder during pm-update drupal Provide a way to not delete files or folders during pm-update drupal Apr 28, 2014
@weitzman
Copy link
Member

Dupe of https://drupal.org/node/1018190. Can you not tell your IDE store settings elsewhere?

@greg-1-anderson
Copy link
Member

There is one thing that we could do here that would help w/out solving all of https://drupal.org/node/1018190. Drush could download the current installed version of the module / theme / Drupal being updated, and remove only those files that appear in the old version, but that do not appear in the new version. This would not cover files like .htaccess that appear in the project, but are modified by the user. (These changes should be thrown away, and 1018190 should therefore be "won't fix", imo.) This would also solve the problem caused by modules like ckeditor, which want their .js files stored in the modules folder instead of someplace more reasonable, like libraries.

See also #4. If we fixed that and applied the new code to all modules, that would also be sufficient, so perhaps we can mark this as a dup of that issue.

@weitzman
Copy link
Member

I don't like the idea of leaving behind files that are no longer used in a project. Its gets confusing to understand what code you are running.

@kgaut
Copy link
Author

kgaut commented Apr 28, 2014

Hi,

thanks for your answer, just to be clear, the files I want drush not to delete aren't related to drupal instance.
If you use phpstorm, it would be the .idea folder, with eclipse or aptana, it's a different folder. Also we could have another folder, like "blog" if we have a wordpress instance at : www.mysite.com/blog/ (it's just a example...)

The state of .htaccess or robots.txt files is different :
Sometimes the .htaccess is modified between two version of drupal, so it's not a drawback to have to manually merge it to our modified version.

@greg-1-anderson
Copy link
Member

@weitzman My proposal was to leave behind only files that were never in the project. So, if there are files in the user's project directory that are not also in the new version of the project, they will only be deleted if they existed in the old version of the project, currently installed.

I won't have time to work on this any time in the near future, but I'd review contributions if the strategy seemed agreeable to everyone.

@AdamPS
Copy link
Contributor

AdamPS commented Jul 17, 2014

I'm hitting this same issue but with a different scenario. I am losing key files for my Drupal site when I update Drupal core using drush.

  1. I create all my Drupal sites with my own customer installation profile. I followed Drupal docs https://www.drupal.org/node/1022020 which says "Profile files should reside in their own directory within the /profiles directory to be loaded.". My installation profile is deleted when I upgrade core.

  2. I put some custom scripts in my own directory in the Drupal root, and these get deleted too. In this case, it could be argued that I should find a better place for them. From the code it looks like it would have to be inside /sites/, so maybe sites/all/scripts would work. However, it would be somewhat tedious for me to change, and in any case I'd still have problem 1).

@greg-1-anderson Your strategy would certainly solve my problem, and seems good. I'm not sure I have the knowledge to code it though.

A potentially simpler solution would be to provide a way to manually add directrories to the $skip_list. I don't know what mechanism would be most compatible with standard drush mechanisms, so would welcome a suggestion.

  • An easy answer that I could code would be to read the contents of a Drupal variable e.g. drush_skip_list.
  • A command line option could also work, although it would be much easier to accidentally forget. I guess this could be mitigated to a degree using drushrc.php.

@greg-1-anderson
Copy link
Member

Well, you shouldn't be upgrading your live site directly in any event; if this is the case, it implies that you have some sort of deploy strategy. Some people like to put all of their Drupal core and contrib code into a git repository, and use git push / git pull to deploy changes, including updates. If you did this, you could run pm-updatecode on your dev site, and then use git checkout -- path to bring back things that Drush should not have deleted. This won't help if you want your profile to be updated, but it's a reasonable workaround for things like scripts directories.

(For what it is worth, I always put my git project root at /srv/www/websitedomain, and I put my Drupal root at /srv/www/websitedomain/htdocs, so I have a nice place right above the Drupal root that is inside of the git project, but outside of the Drupal root for storing files that should not be visible to the webserver.)

I'm not very excited about the skip list idea. I suppose it would work, but I'd rather see pm-update fixed in a more robust way. Maybe I'll get to this at some point, but I don't have time for it in the near future.

@AdamPS
Copy link
Contributor

AdamPS commented Jul 17, 2014

Thanks for the reply Greg. I don't update my live sites directly, and end up doing much the same as you say - repair the test sites from code management. Your htdocs idea is a sensible one and with hindsight I might have done that, but hopefully you won't deliberately code Drush assuming that particular approach.

It sounds like I should roll my own solution for now (I guess I can just hard-code the extra skip directories on my local copy of drush!), and if you do ever write the robust approach that's clearly better.

@greg-1-anderson
Copy link
Member

Of course the robust approach would be best, but short of that, a PR that added items to the skip list would be an improvement.

@AdamPS
Copy link
Contributor

AdamPS commented Jul 28, 2014

When I looked into it closer, the skip list approach wasn't as useful as I had anticipated. As far as I could tell it is only possible to skip top level directories. I.e. it solves my second problem but not the first - and the first seems more likely for others to hit as that's a Drupal mandated directory location.

@superbiche
Copy link

This only applies to the .htaccess issue (that was one of the main topic of https://drupal.org/node/1018190): couldn't we write some of the directives we want to keep when upgrading core between .htaccess comment tags, something like # and #, and add a Drush behavior to catch this block and its contents, the rewrite it in the upgraded .htaccess?
This would make the upgrade process safer, avoid extra manipulation/hooks, and prevent not upgrading .htaccess, which isn't safe either.

Can try to make a PR for this.

@AdamPS
Copy link
Contributor

AdamPS commented Nov 10, 2015

@superbiche Your idea sounds fine, but it is not really part of this issue (maybe more the one you reference?). This issue is talking about avoiding deleting files that weren't put there by Drupal in the first place. Drupal does provide .htaccess.

Maybe best if you create a new issue for your idea, or use another one that is is more closely related to continue your discussion?

@superbiche
Copy link

In the issue I reference, @greg-1-anderson asked to move the debate about this potential feature here on Github. So I'll open a new one here.
Sorry for the inconvenience :)

@philsward
Copy link

Would it be worth having a .drushignore file within the root of Drupal and anything within that file is ignored?

This would allow the .htaccess and robots.txt issue to be handled a bit better. I realize the existing Drupal files are supposed to be on another issue, but they do all kind of come together. Personally, I think one issue should cover all of the use cases until a clear understanding of how to approach each use case is determined, then they can be split off to separate issues.

Right now we have:

  • Drupal files vs Non-Drupal files
  • Is it possible for Drush to actually ignore files or would they have to be copied out of root and copied back?
  • If Drupal files, how do we deal with existing files? Merge? Overwrite? Rename New, Rename Old?

I can see Drush asking the user what they want to do with each existing file. Since Drush does a backup by default, if the user fat-fingers their choice, they can always retrieve out of the backup, right?

I've got some other thoughts on the existing Drupal files, so I'll go find that issue to post in.

Update: Never found the other issue...

I still ftp my drupal updates. Why? Because I can control what gets updated. Whenever I download the update and extract it, I rename .htaccess and robots.txt on local, then upload. I don't want those files overwritten with the updated version because I know I will need to manually merge any info that has changed. I have 20+ sites that need to be updated and I don't have time to remember which files I need to deal with. Rename it once on the local and then blow it down to all of the sites. It would save a huge amount of time if I could tell drush to ignore the .htaccess and robots.txt file for each site. I could drush update core and be done in 10 seconds instead of the 5 - 10 it takes me now to ftp.

@AdamPS
Copy link
Contributor

AdamPS commented Dec 5, 2015

Note that drush expert @greg-1-anderson has already proposed a solution that I think is extremely attractive: drush can automatically determine which files were part of drupal in the first place and only delete those same files.

As remarked earlier in the thread, a .drushignore file could be better than nothing for the user to manually set which files are ignored (which yes I expect means copied out and copied back).

In terms of this thread, the above defines a strategy which has a rough consensus and it mostly needs someone to code it.

@philsward Please I feel you are hijacking this issue which has a specific topic. You can create your own issue to discuss editing Drupal files, or a meta-issue to link several topics if you like. If you need advice, try Drupal Answers http://drupal.stackexchange.com/.

Note that editing core files is considered inadvisable by many. I'm not sure there is likely to be a general solution because a merge is needed which likely requires manual intervention. Many techniques exist already (but I'd rather not discuss them, further right here!):

@greg-1-anderson
Copy link
Member

We'd need a pm-update champion for this to move forward. I am putting my efforts towards updating via Composer; see drupal-composer/drupal-project#77 and drupal-composer/drupal-scaffold. I personally would just as soon see Drush get out of the updating business, but I know it will be a long time before everyone is using Composer.

@AdamPS
Copy link
Contributor

AdamPS commented Feb 18, 2016

It looks like code to solve some of all of these problems is now in D8. Maybe best just to close this one as fixed?

BTW I am a strong fan of drush updating - it works and I don't have to learn a new tool like composer.

@greg-1-anderson
Copy link
Member

I don't disagree about the usefulness of the pm-* code; we just need someone to step forward and maintain it if it is going to be maintained.

@AdamPS
Copy link
Contributor

AdamPS commented Feb 18, 2016

@greg-1-anderson I will do what I can to help keep pm-* code alive, but I'm certainly not a drush expert. In fact there's action right now on #1056 where I'm keen to help but it needs a drush expert involved for advice - if you can help or know someone who can, I would be grateful.

In the meantime, I think we should close this issue because it's fixed (or almost all) in Drush 8. But I can't see a way for me to do it, so please could you?

@greg-1-anderson
Copy link
Member

I haven't confirmed that these issues have been fixed in Drush 8. Are there unit tests covering this functionality in place?

@AdamPS
Copy link
Contributor

AdamPS commented Feb 18, 2016

Sorry, sure if you are still working on it then no problem. No I haven't checked the unit tests, I'm afraid I don't even know where they live.

By real use experiment, I have found that 'unexpected' files in the top level directory and in the profiles directory are preserved when drupal core is upgraded. This is achieved without the need for user configuration by finding any files that are missing present in the new drupal core and preserving them. Non-writable files and recognised code management files are also preserved.

@greg-1-anderson
Copy link
Member

@jhedstrom probably knows and can close if appropriate.

@AdamPS
Copy link
Contributor

AdamPS commented Feb 22, 2016

Update - I'm not sure this is fixed. The new D8 code seems to preserve any 'unexpected' file in the root. Unfortunately I think this would also preserve a file that was in Drupal core then got removed in a newer version. Unlikely, but worse case potentially it could back out a security fix. A second problem is that the fix appears to be only for VCS=backup. I'll raise a new issue when I have a mo.

@AdamPS
Copy link
Contributor

AdamPS commented Feb 23, 2016

New issue raised. First step would be useful if anyone could comment on whether it's a good idea.

If so I could try to code it, but it's a lower priority than a bunch of real bugs I'm hitting now I'm on drush 8 - if anyone from the drush team gets a chance to look at those issues and the pull request I'd be grateful.

@greg-1-anderson the composer framework looks interesting and I might look into it when I move to D8, but that's probably not for a couple of years for me.

@AdamPS
Copy link
Contributor

AdamPS commented Feb 10, 2017

To summarise: this is fixed in the default scenario, which is

  • --version-control=backup
  • There is a backup (i.e. not --no-backup)

@weitzman stated in #2275 that he "doubt this will ever get fixed" to work with --no-backup (I did have a PR but it was not taken - significant changes to drush pm are not wanted any more as pm is deprecated.)

So I suggest it makes sense to close this issue as fixed - which is true for 99% of users.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants