For the Aegir project it would be usefull to create archives of just the site specifics.
Therefore --no-core was opted in #1138882: Support drush archive-dump

I've started some work on this in a sandbox, dump seems to be working.
I've added an extra testCase for this functionality,

See: http://drupalcode.org/sandbox/helmo/1277350.git

The current method of restore is an issue though as drush_move_dir() is used which does not like an existing target directory. This use-case would need a 'copy' method.

The sandbox also contains a few commits with general cleanup.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

moshe weitzman’s picture

Status: Active » Needs work

Looks good. I think you can just run with --no-core in the existing test and check that all looks good. I'm OK with creating a new test, but I fail to see why we need to clear the sandbox between tests. Can't we just generate a second archive into a different --destination?

Lets discuss restore in a separate issue.

helmo’s picture

Including this test in testArchiveDump() would make it longer then I would prefer for a method.
I also have the desire to extend this even further to be able to archive just a platform, without any sites. Then it's just easier to add a third case then to again lengthen this one.

The reason i chose to clear the sandbox was that it's not only the dump filename that conflicts but also the untar directory and the $expected string.
To get the cleanest possible test, while preserving some cached data, this seemed like the best idea,

I'd suggest calling a new method e.g. setUpFreshSandBox() instead of setUpBeforeClass().
setUpBeforeClass() would then delegate all it's work to this new method.

moshe weitzman’s picture

It sounds like archive-dump is not cleaning itself up properly after making an archive? I don't see why the sandbox should be polluted with anything but a single tarball.

Anyway, refactoring into setUpFreshSandBox() sounds good. Feel free to put it into this patch.

helmo’s picture

I pushed the refactorring of setUpFreshSandBox() into my sandbox.

The 'untar' directory is a result from test code, so not a problem of archive-dump itself.

moshe weitzman’s picture

Well, lets clean up after ourselves in the unit test code then. We can keep the refactoring but I don't want to encourage a pattern where methods are clearing sandbox 'just in case'. Lets not actually call the new method.

helmo’s picture

Status: Needs work » Needs review

ok, it's now cleaning up in the test itself.

I've also opened an issue for the archive-restore part: #1280218: archive-restore adaption for an archive made with --no-core option

moshe weitzman’s picture

You can't call a drush function directly from a functional test. You can do it from a pure unit test since those do bootstrap drush in same process. I'm surprised (and worried) that you are not seeing a fatal when running just this test.

Instead call $this->drush('eval', ' drush_delete_tmp_dir()l')

helmo’s picture

Eval... :( no.

Gladly I found unish_file_delete_recursive() as a replacement function.

anarcat’s picture

@moshe - i thought we could call functions directly in unit tests now that the unit testing framework was rewired?

moshe weitzman’s picture

Please post a patch instead of (or in addition to) a link to your sandbox. i would like to review the diff but not having it easily available is a deterrent.
@anarcat - sure, thats right. IIRC, the patch here did not use a unit test but rather a traditional system test.

moshe weitzman’s picture

Priority: Normal » Major

I'd like to get this committed for Aegir.

moshe weitzman’s picture

Status: Needs review » Postponed (maintainer needs more info)
helmo’s picture

Status: Postponed (maintainer needs more info) » Needs review
FileSize
11.61 KB

Here's the patch from a diff of origin/master..sandbox/master-archive-dump

helmo’s picture

I recently discovered a way to link to a diff from drupalcode.org.
This link shows all changes in my branch: http://drupalcode.org/sandbox/helmo/1277350.git/commitdiff/master-upstre...

And as plain text: http://drupalcode.org/sandbox/helmo/1277350.git/commitdiff_plain/master-...

I cleaned up some whitespace errors in the meantime....

mortona2k’s picture

I would really like to use --no-core, and I am having trouble applying the patch. I am using drush 5 rc 3.

EDIT: I just copied your archive.drush.inc file from git, and everything seems to be working smoothly. Thanks!

moshe weitzman’s picture

Status: Needs review » Needs work

That diff is now huge and contains unrelated changes

helmo’s picture

Not really surprising that the patch no longer applies...
Is there a realistic chance of this getting in anytime soon? If so then I'll see if I can re-roll.

helmo’s picture

After all this time... I've rebased.

I've split off a some cleanup changes that are beneficial to this patch(also in my sandbox)
I can make a separate issue for it if you really like, but they should be easy to review/accept.

Please review to get this finally in(sandbox branch).

Tests from archiveDumpTest.php still run OK after this.

helmo’s picture

I created a separate issue for the cleanup patch: #1616416: Cleanup patch

helmo’s picture

As the cleanup patch has been committed to master, I've updated the rest with one comment from @moshe weitzman

vinmassaro’s picture

Would like to see this as well but the patch in #20 did not apply cleanly for me.

helmo’s picture

OK, I've re-rolled the patch

vinmassaro’s picture

Patch applies cleanly and works great for me. Produces a database and the sites/default directory.

vinmassaro’s picture

Status: Needs review » Reviewed & tested by the community
mortona2k’s picture

Applies cleanly to 5.6 as well.

vinmassaro’s picture

Will this make it into 7.x-5.8?

vinmassaro’s picture

@helmo, can you reroll the patch against 7.x-5.x HEAD? Mentioned this issue on IRC in #drush, hoping to get maintainer review so this can be committed. Thanks!

helmo’s picture

I'd love to (again) but the lack of progress is very discouraging...

The related issue #1280218: archive-restore adaption for an archive made with --no-core option has gone over a year without any review :(

helmo’s picture

@vinmassaro: Could you maybe help in http://groups.drupal.org/site-archive-format to get a page up with a definitive format description?
I've tried to in #1294632: Document Site Archive Format (my draft)

vinmassaro’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
11.48 KB

@helmo, not sure how I can hep in #29 - your draft is looking pretty good.

I followed the patch re-roll guide and was able to get it updated against HEAD (correctly, I hope).

jonhattan’s picture

Status: Needs review » Needs work

Patches must be against 6.x-dev branch. Anyway here's a preliminary review:

+++ b/commands/core/archive.drush.inc
@@ -21,6 +21,7 @@ function archive_drush_command() {
+      'no-core' => 'Exclude Drupal core, so include just site specific stuff.',

Let's be more precise in the description.

I guess this option just add sites/default or sites/example.com to the tarball.. Isn't it?

+++ b/commands/core/archive.drush.inc
@@ -214,6 +224,7 @@ function drush_archive_dump($sites_subdirs = '@self') {
+    'archive_format' => ($include_platform ? 'platform' : 'site'),

Other fields in the manifest are: formatversion, generator, generatorversion. In concordance the new field should be archiveformat.

Also platform is an Aegir word. I'm fine with it but perhaps there's a better term.

+++ b/commands/core/archive.drush.inc
@@ -316,13 +309,27 @@ function drush_archive_restore($file, $site_id = NULL) {
+      return drush_set_error('DRUSH_ARCHIVE_UNABLE _RESTORE_FILES', dt('Unable to restore platform to !dest', array('!dest' => $destination)));

That whitespace in the error code is not introduced by this patch, but just seen it now. It should be DRUSH_ARCHIVE_UNABLE_TO_RESTORE_FILES. Feel free to fix it, or I'll fix it once this issue lands.

+++ b/commands/core/archive.drush.inc
@@ -390,3 +397,49 @@ function archive_archive_restore_complete() {
+          // Very crude detection of a platform...
+          'archive_format' => (file_exists($directories . '/modules') ? 'platform' : 'site'),

A more consistent way: drush_drupal_version($docroot)

vinmassaro’s picture

Status: Needs work » Needs review
FileSize
12.53 KB

@jonhattan, rerolled against 8.x-6.x and includes your suggested changes from #31 except for changing the term 'platform'. Wasn't sure how to add credit for helmo since he created the original patch. Thanks!

jonhattan’s picture

I've reworked some parts of the patchin #32, including my own suggestions in #31 and a major refactor of the tests. I'm attaching the interdiff and the full patch for reference.

jonhattan’s picture

Status: Needs review » Fixed

Committed to 6.x.

vinmassaro’s picture

This is great, thanks! How does this make it into 7.x-5.x for the next Drush release?

greg.1.anderson’s picture

Status: Fixed » Patch (to be ported)

Someone would need to backport this to the 5.x branch. Setting status for consideration.

jonhattan’s picture

I'm for backport, but the patch doesn't apply because of the commits done within #1844224: Document how to correctly format filesystem paths for Windows shells and insure existing usage is consistent and I'm not sure how to proceed.

greg.1.anderson’s picture

I think that #844224 could be backported to 5.x as well.

vinmassaro’s picture

I re-rolled #844224 against 7.x-5.x and posted it in that issue, and here is a re-roll of this patch against 7.x-5.x, as well as a combined patch from both issues. I believe I did this correctly, but if this is a mess, please let me know!

jonhattan’s picture

Status: Patch (to be ported) » Fixed

backported.

@vinmasaro thanks for the patchs. That's not neccesary since the 6.x version can be applied to 5.x once #1844224: Document how to correctly format filesystem paths for Windows shells and insure existing usage is consistent was backported.

vinmassaro’s picture

Just wanted to follow-up with how this is helping us at Yale. We allow site developers to fork our main d7 repo, and work out of their fork. Commits to their forked repo trigger a Jenkins build that deploys the updated code to their site. At the beginning of each build, drush archive-dump runs to create a backup. This can potentially add a substantial amount of time to the build, depending on the size of the database and files, as well as git data.

Using --no-core, we are able to drop a lot of time from builds, and this is especially helpful if developers are making small iterative commits.

On a site with ~100mb of files in the tree (including git data) and ~50mb uploaded files running on MAMP as a test, we see an ~88% decrease in archive-dump time:

Vincents-iMac:d7 vincentmassaro$ time drush archive-dump
...  

Archive saved to /Users/vincentmassaro/drush-backups/archive-dump/20130109165647/d7.20130109_045647.tar.gz     [ok]

real	0m28.016s
user	0m7.842s
sys	0m1.495s

Vincents-iMac:d7 vincentmassaro$ time drush archive-dump --no-core
...             

Archive saved to /Users/vincentmassaro/drush-backups/archive-dump/20130109165817/d7.20130109_045817.tar.gz     [ok]

real	0m3.365s
user	0m2.265s
sys	0m0.275s

As well as a ~71% decrease in disk space used:

Vincents-iMac:d7 vincentmassaro$ du -sk ~/drush-backups/archive-dump/20130109165647
153652	/Users/vincentmassaro/drush-backups/archive-dump/20130109165647
Vincents-iMac:d7 vincentmassaro$ du -sk ~/drush-backups/archive-dump/20130109165817
44984	/Users/vincentmassaro/drush-backups/archive-dump/20130109165817
helmo’s picture

Thanks all for getting this committed.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.