Add documentation advising against using gsutil rsync or cp over directories containing symlinks

Add documentation advising against using gsutil rsync or cp over directories containing symlinks

[Mike Schwartz, 2018-09-22 20:14:31]

Replacing https://codereview.appspot.com/342260043/

[nstock, 2018-09-24 17:08:27]

https://codereview.appspot.com/353780043/diff/1/gslib/commands/rsync.py
File gslib/commands/rsync.py (right):

https://codereview.appspot.com/353780043/diff/1/gslib/commands/rsync.py#newco...
gslib/commands/rsync.py:231: We recommend that customers either:
Formatting nit - I'd suggest writing this as:

We recommend that users do one of the following:

* Not use gsutil rsync over directories containing symlinks.
* Use the -e option to exclude symlinks.
* Use a tool (such as tar) that preserves symlinks and package up
 directories containing symlinks before uploading to the cloud.

https://codereview.appspot.com/353780043/diff/1/gslib/commands/rsync.py#newco...
gslib/commands/rsync.py:233: * not use gstil rsync over directories containing
symlinks;
gsutil

[houglum, 2018-09-24 17:59:20]

https://codereview.appspot.com/353780043/diff/1/gslib/commands/rsync.py
File gslib/commands/rsync.py (right):

https://codereview.appspot.com/353780043/diff/1/gslib/commands/rsync.py#newco...
gslib/commands/rsync.py:212: the symlinks point. In most cases this is not the
behavior you would want. A
s/In most cases/In some cases/

Saying gsutil's behavior isn't the behavior you normally want seems... like we
just shouldn't have decided on that behavior in the first place.

[Mike Schwartz, 2018-09-24 18:21:10]

https://codereview.appspot.com/353780043/diff/1/gslib/commands/rsync.py
File gslib/commands/rsync.py (right):

https://codereview.appspot.com/353780043/diff/1/gslib/commands/rsync.py#newco...
gslib/commands/rsync.py:212: the symlinks point. In most cases this is not the
behavior you would want. A
On 2018/09/24 17:59:20, houglum wrote:
> s/In most cases/In some cases/
> 
> Saying gsutil's behavior isn't the behavior you normally want seems... like we
> just shouldn't have decided on that behavior in the first place.

I don't think I agree with the rewording or the 'in the first place' comment.
The point we're making here is that gsutil wasn't intended to be used over
symlinks. I think that's a perfectly valid design choice. It's also not intended
to be used over dev files or socket files.

The way I see it, people can create files in a local file system that don't make
sense either in the cloud or when copying files to a different operating system.
But that doesn't mean we shouldn't provide a tool intended for the subset of
things that make sense in the cloud and cross-OS. That's exactly what gsutil
rsync is.

I'm open to a different way to explain the point, but I don't think we should
pretend symlinks might work ok for some cases by doing s/In most cases/In some
cases.

https://codereview.appspot.com/353780043/diff/1/gslib/commands/rsync.py#newco...
gslib/commands/rsync.py:231: We recommend that customers either:
On 2018/09/24 17:08:27, nstock wrote:
> Formatting nit - I'd suggest writing this as:
> 
> We recommend that users do one of the following:
> 
> * Not use gsutil rsync over directories containing symlinks.
> * Use the -e option to exclude symlinks.
> * Use a tool (such as tar) that preserves symlinks and package up
>  directories containing symlinks before uploading to the cloud.

Done.

https://codereview.appspot.com/353780043/diff/1/gslib/commands/rsync.py#newco...
gslib/commands/rsync.py:233: * not use gstil rsync over directories containing
symlinks;
On 2018/09/24 17:08:27, nstock wrote:
> gsutil

Done.

[houglum, 2018-09-24 19:41:56]

https://codereview.appspot.com/353780043/diff/1/gslib/commands/rsync.py
File gslib/commands/rsync.py (right):

https://codereview.appspot.com/353780043/diff/1/gslib/commands/rsync.py#newco...
gslib/commands/rsync.py:212: the symlinks point. In most cases this is not the
behavior you would want. A
On 2018/09/24 18:21:10, Mike Schwartz wrote:
> On 2018/09/24 17:59:20, houglum wrote:
> > s/In most cases/In some cases/
> > 
> > Saying gsutil's behavior isn't the behavior you normally want seems... like
we
> > just shouldn't have decided on that behavior in the first place.
> 
> I don't think I agree with the rewording or the 'in the first place' comment.
> The point we're making here is that gsutil wasn't intended to be used over
> symlinks. I think that's a perfectly valid design choice. It's also not
intended
> to be used over dev files or socket files.
> 
> The way I see it, people can create files in a local file system that don't
make
> sense either in the cloud or when copying files to a different operating
system.
> But that doesn't mean we shouldn't provide a tool intended for the subset of
> things that make sense in the cloud and cross-OS. That's exactly what gsutil
> rsync is.
> 
> I'm open to a different way to explain the point, but I don't think we should
> pretend symlinks might work ok for some cases by doing s/In most cases/In some
> cases.

We could probably just cut this sentence out. I don't think it adds anything
meaningful to the section that isn't already conveyed.

[Mike Schwartz, 2018-09-24 20:00:26]

https://codereview.appspot.com/353780043/diff/1/gslib/commands/rsync.py
File gslib/commands/rsync.py (right):

https://codereview.appspot.com/353780043/diff/1/gslib/commands/rsync.py#newco...
gslib/commands/rsync.py:212: the symlinks point. In most cases this is not the
behavior you would want. A
On 2018/09/24 19:41:55, houglum wrote:
> On 2018/09/24 18:21:10, Mike Schwartz wrote:
> > On 2018/09/24 17:59:20, houglum wrote:
> > > s/In most cases/In some cases/
> > > 
> > > Saying gsutil's behavior isn't the behavior you normally want seems...
like
> we
> > > just shouldn't have decided on that behavior in the first place.
> > 
> > I don't think I agree with the rewording or the 'in the first place'
comment.
> > The point we're making here is that gsutil wasn't intended to be used over
> > symlinks. I think that's a perfectly valid design choice. It's also not
> intended
> > to be used over dev files or socket files.
> > 
> > The way I see it, people can create files in a local file system that don't
> make
> > sense either in the cloud or when copying files to a different operating
> system.
> > But that doesn't mean we shouldn't provide a tool intended for the subset of
> > things that make sense in the cloud and cross-OS. That's exactly what gsutil
> > rsync is.
> > 
> > I'm open to a different way to explain the point, but I don't think we
should
> > pretend symlinks might work ok for some cases by doing s/In most cases/In
some
> > cases.
> 
> We could probably just cut this sentence out. I don't think it adds anything
> meaningful to the section that isn't already conveyed.

Agreed. Done.

I generalized this section to cover other OS-specific file types.

[houglum, 2018-09-24 20:06:50]

On 2018/09/24 20:00:26, Mike Schwartz wrote:
> https://codereview.appspot.com/353780043/diff/1/gslib/commands/rsync.py
> File gslib/commands/rsync.py (right):
> 
>
https://codereview.appspot.com/353780043/diff/1/gslib/commands/rsync.py#newco...
> gslib/commands/rsync.py:212: the symlinks point. In most cases this is not the
> behavior you would want. A
> On 2018/09/24 19:41:55, houglum wrote:
> > On 2018/09/24 18:21:10, Mike Schwartz wrote:
> > > On 2018/09/24 17:59:20, houglum wrote:
> > > > s/In most cases/In some cases/
> > > > 
> > > > Saying gsutil's behavior isn't the behavior you normally want seems...
> like
> > we
> > > > just shouldn't have decided on that behavior in the first place.
> > > 
> > > I don't think I agree with the rewording or the 'in the first place'
> comment.
> > > The point we're making here is that gsutil wasn't intended to be used over
> > > symlinks. I think that's a perfectly valid design choice. It's also not
> > intended
> > > to be used over dev files or socket files.
> > > 
> > > The way I see it, people can create files in a local file system that
don't
> > make
> > > sense either in the cloud or when copying files to a different operating
> > system.
> > > But that doesn't mean we shouldn't provide a tool intended for the subset
of
> > > things that make sense in the cloud and cross-OS. That's exactly what
gsutil
> > > rsync is.
> > > 
> > > I'm open to a different way to explain the point, but I don't think we
> should
> > > pretend symlinks might work ok for some cases by doing s/In most cases/In
> some
> > > cases.
> > 
> > We could probably just cut this sentence out. I don't think it adds anything
> > meaningful to the section that isn't already conveyed.
> 
> Agreed. Done.
> 
> I generalized this section to cover other OS-specific file types.

I like how this reads much better :) Thanks!
LGTM

[nstock, 2018-09-24 22:21:25]

LGTM

[Mike Schwartz, 2018-10-12 23:00:56]

Please disregard. This diff belongs to a difference CL (
https://codereview.appspot.com/349790043)

On Fri, Oct 12, 2018 at 4:06 PM, <mfschwartz@google.com> wrote:

> https://codereview.appspot.com/353780043/
>