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

High risk of data loss on collaboration #57

Closed
FPtje opened this issue Feb 5, 2015 · 10 comments
Closed

High risk of data loss on collaboration #57

FPtje opened this issue Feb 5, 2015 · 10 comments

Comments

@FPtje
Copy link

FPtje commented Feb 5, 2015

From rakyll#59

Do NOT use this application when you're working with classmates on a project. The risk of losing data in files is way too damn high. Here's why:

The merging mechanism isn't just stupid, it's nonexistent. Have this situation:

a/
   b.txt
   c.txt

Have one example. It's the absolute worst one:

  1. Edit b.txt
  2. drive pull a
  3. b.txt will show an "M", stating it's modified. If you proceed with pulling, b.txt will be overwritten with an older file.

Why would you want to pull the folder? Well, your teammates might have edit c.txt, or maybe they have even edited b.txt. When you see that b.txt was modified, you're led to believe that your teammates modified it. The fact is that it could mean that, but it's probably because you edited b.txt yourself. There is no way to tell.

In a project I handed in today I was very careful to push and pull per file. Luckily no one else edited files at the same time I did, that would have put me in deep trouble. That or maybe that did happen, drive wouldn't have told me so there would be no way for me to find out unless they actually told me. Despite the per file push/pulling I still lost data, most likely because I pulled the folder thinking I had pushed my per file changes.

Files just get overwritten. Both on push and on pull. No merging, no conflict mechanism, no warnings that you're overwriting unsaved changes (or haven't merged with the latest version yet) and a downright misleading status message. This tool is not safe for projects in which people cooperate.

Here's the example of what I'm talking about:
given example

Tested on the AUR version of drive (0.0.8-1 (Thu Feb 5 18:57:44 CET 2015))

@odeke-em
Copy link
Owner

odeke-em commented Feb 5, 2015

Too bad that this happened to you, I saw the issue you raised of rakyll's project and that was using her version right?
To help you mitigate such issues, if you go through the README of this project

$ drive pull -no-clobber # Non-clobberable pull?
$ drive diff vegetable.text # See the differences

From the README
screen shot 2015-02-05 at 11 09 44 am
In deed, this project doesn't do any conflict resolution and in previous issues this had been outlined in
rakyll#49. In fact it is up to the user to decide if they want the changes or not as seen in your screenshot where the remote was 7Bytes and local was 5Bytes. If this were git you wouldn't even be prompted for, an automatic merge would have been made. If you want concurrent editing, I'd recommend using Google Docs.
Also I thought the intent was pretty obvious from reading the description of the README and project, pull or push Google Drive files.
Please let me know what you think.

@FPtje
Copy link
Author

FPtje commented Feb 5, 2015

No, I tested on the AUR version of drive (0.0.8-1)

The no-clobber leads to an error when it's the last parameter:

falco@manjaro ~/Drive> drive pull vegetable.txt -no-clobber
Resolving...
remote path doesn't exist

falco@manjaro ~/Drive [1]> drive pull vegetable.txt -no-clobber=true
Resolving...
remote path doesn't exist

falco@manjaro ~/Drive [1]> drive pull vegetable.txt 
Resolving...
M /vegetable.txt

It does do the job when running drive pull -no-clobber vegetable.txt.

The no-clobber function is deeply hidden away in the drive program. The readme doesn't mention it. It should be default behaviour. One could never reasonably expect that drive will overwrite modified files on pulls. extreme caution is to be taken when pulling. This contrasts with git, where a pull would fail when local files would be overwritten. With git you never lose data:

error: Your local changes to the following files would be overwritten by merge:
    gamemode/modules/chat/sv_chat.lua
Please, commit your changes or stash them before you can merge.
Aborting

This application's behaviour is really confusing. Having it behave in a logical way requires the use of a parameter that has a very odd name (clobber?) and is hidden deeply in the documentation.

This is mostly a usability problem. The program should have proper default behaviour, and people's data should be considered sacred. Accidental data loss should not be possible with a simple drive pull command.

@odeke-em
Copy link
Owner

odeke-em commented Feb 5, 2015

You are using it wrong

$ drive pull -no-clobber vegetable.txt

instead of

$ drive pull vegetable.txt -no-clobber

Yeah the CLI args parser only parses options before the arguments.

Don't you get a warning that asks whether you want to proceed with the modifications?
Also git has the ability to merge, if a merge fails it aborts. This tool doesn't have that so far.
That was the purpose of -no-clobber as well -force

@FPtje
Copy link
Author

FPtje commented Feb 5, 2015

Like I said, it works fine when doing drive pull -no-clobber vegetable.txt

With -no-clobber, drive doesn't mark the file as changed and is thus not overwritten.
The usability issue still stands.

@FPtje
Copy link
Author

FPtje commented Feb 5, 2015

Look, you don't have to have advanced merge functionalities, these are hard to write. A conflict mechanism like in the Windows drive client and in dropbox shouldn't be too hard to make.

If remote file modified AND local file modified then abort with error 
or
If remote file modified AND local file modified then download remote file as '(CONFLICT) filename'

The hardest part of that should be detecting whether a file was locally modified. You already have that logic in the drive push command.

A simple error would have saved my data.

@odeke-em
Copy link
Owner

odeke-em commented Feb 5, 2015

That is true, except we don't have the ability to perform merges yet, this tool right performs a naive pull and push and cannot detect when merging has failed. It also provides a prompt if 'naive' clobbering is being done. Feel free to submit a PR with merging and conflict detection if you'd like. If you'd like pointers on this, I started a branch https://github.com/odeke-em/drive/tree/change-tracking that will able to track changes on files and avoid such problems, but then again I didn't want to mention this until I fully understood the problem and had provided a full implementation.

@odeke-em
Copy link
Owner

odeke-em commented Feb 5, 2015

I see what you are saying. For sure, sounds good.

@odeke-em
Copy link
Owner

odeke-em commented Feb 5, 2015

I'll get this in when I get home. Thanks for the discussion, that was actually a good idea of throwing the error.

@FPtje
Copy link
Author

FPtje commented Feb 5, 2015 via email

@odeke-em
Copy link
Owner

odeke-em commented Feb 7, 2015

Got in the level one conflict detection mechanism. Feel free to close this if the current solution fulfills the purpose. A better version will be coming once I incorporate change tracking to tell when the local has been modified too and then drive can really detect when a real conflict is present.

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

2 participants