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

Correctly handle directories that are actually custom stream wrappers #538

Open
pdewouters opened this issue Sep 1, 2014 · 10 comments
Open

Comments

@pdewouters
Copy link
Contributor

For example, if the site is using the S3 uploads plugin to store media on Amazon S3, BWP complains:

BackUpWordPress is almost ready. The backups directory can't be created because your wp-content directory isn't writable, run chown www-data:www-data s3:/hmn-uploads/newton/uploads or chmod 777 s3:/hmn-uploads/newton/uploads or create the folder yourself.

s3:/hmn-uploads/newton/uploads is writable, but I think probaly fails the PHP function we are using

@pdewouters pdewouters added the Bug label Sep 1, 2014
@pdewouters pdewouters added this to the Awaiting Review milestone Sep 1, 2014
@willmot
Copy link
Contributor

willmot commented Dec 18, 2014

What should we be doing in this instance? Is it possible for us to tell whether something like an S3 url is writable / correct? Or should we only be showing those warnings if the backup directory is on local filesystem?

@pdewouters pdewouters modified the milestone: Awaiting Review Feb 14, 2015
@willmot
Copy link
Contributor

willmot commented Jul 17, 2015

cc @joehoyle would be good to get your input here

@joehoyle
Copy link
Contributor

After looking into the whole BUWP + S3 Uploads thing, the main problem is that BUWP is not going to support an uploads directory using a custom stream handler (that's to say anything other than the filesystem stream handler.)

I'd say you should have an activated check or whatever that the uploads dir is a local stream wrapper, and fail if it is not. This error is just the beginning of what the problems will currently be even if suppressing this error.

@willmot
Copy link
Contributor

willmot commented Jul 18, 2015

Makes sense, I'm +1 on detecting and warning the user. Does the fact the the uploads folder is using a custom stream wrapper always mean we need to fail though or will it only fail if we are trying to store backups in the uploads directory?

If the backup is stored elsewhere on the filesystem it should work fine right? In that case, are we able to backup the uploads directory if it's using a custom stream wrapper? Or does it stop us accessing the files for the purposes of backup also?

I think we have two possible scenarios both of which need detecting and reporting to the user.

  1. Your backups directory couldn't be created in WP_CONTENT_DIR and thus tried to fallback to creating it in uploads but couldn't because it's using a custom stream wrapper. Because of this we can't perform backups until you manually define a valid backups directory. This is basically the same as the generic, we couldn't create you a backups directory error message we already have.
  2. Your backups are stored elsewhere so that's fine, but we're unable to backup your uploads folder (or whatever folder) because it's using a stream wrapper.

Make sense?

@willmot willmot added this to the Future Release milestone Jul 18, 2015
@joehoyle
Copy link
Contributor

Right, so if your using a custom stream wrapper, I guess there's no reason you can't store the backups in /tmp or whatever, however if you do that, I don't know how backupwordpress supporting downloading those backups.

However, if you're not backing up the uploads, I'm not sure of the utility for anything other than database backups.

The current system won't work with custom stream wrappers as we're using system tools like zip to backup the entire directory, however I outlined in #574 how I think that would work.

@willmot
Copy link
Contributor

willmot commented Jul 18, 2015

@joehoyle ok cool, makes sense 👍

For now I'm tempted to just catch and treat uploads as a folder that can't be read and backed up in the same we treat any unreadable files / directories. At least it will then be clear to the user that uploads isn't being backed up.

@willmot willmot changed the title Mistakenly reports backups dir is unwritable if files are stored remotely Correctly handle directories that are actually custom stream wrappers Jul 18, 2015
@willmot
Copy link
Contributor

willmot commented Nov 30, 2015

We should re-test this now we're using Finder as it may just support streams

@willmot willmot added Enhancement and removed Bug labels Mar 11, 2016
@willmot
Copy link
Contributor

willmot commented Sep 3, 2016

Finder does support streams, but looks like we'll still need some special handling.

@libby-barker
Copy link
Collaborator

@willmot can you speak more to the special handling we'd need to include?

@willmot
Copy link
Contributor

willmot commented Nov 4, 2016

I haven't done the work to figure out how and why the the plugin fails when paired with S3 uploads, just that it does.

  • Test the plugin with S3 Uploads
  • Note it doen't work
  • Figure out why
  • Figure out how we can work around it.

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

4 participants