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

Add backup/restore #2747

Closed
Rick-74 opened this Issue Dec 5, 2017 · 29 comments

Comments

Projects
None yet
6 participants
@Rick-74

Rick-74 commented Dec 5, 2017

With old greasemonkey an export scripts/preferences function wasn't really needed, since the user could just move the gm_scripts directory from one profile/computer to another: but now this is no longer possible, so please consider adding such a function to greasemonkey 4 in the future.

@arantius arantius added this to the 4.x milestone Dec 5, 2017

@Sxderp

This comment has been minimized.

Contributor

Sxderp commented Dec 21, 2017

Well, Firefox and its issues. So, I was implementing this (and of course an 'import' function). I'm almost done with it, but there's a show-stopper. It only works when you open the browser toolbox and select "do not autoclose popups". The import is done through a <input type="file"> (which is the only way), however, on focus of the browse window the popup normally closes, thus the window is destroyed and no functions are executed. Relevant bugs:
https://bugzilla.mozilla.org/show_bug.cgi?id=1292701
https://bugzilla.mozilla.org/show_bug.cgi?id=1366330

It's so annoying having a half-assed environment to work in.

Also, applies to #2612. Which can be addressed at the same time this issue is.

@makyen

This comment has been minimized.

makyen commented Dec 21, 2017

@Sxderp If the popup closing is an issue, the normal way to solve/work-around that is to just open a tab or window (pointing to an HTML page within the extension) in which you perform whatever you need to do. Such tabs/windows are not auto-closed and present the same environment as a popup. While that might not be as clean as having it all within the browser popup, it should be functional.

@Sxderp

This comment has been minimized.

Contributor

Sxderp commented Dec 21, 2017

I could do that. It'd involve moving a bunch of code around. I'll look into it some other time.

I've completed the import / export stuff. No tests, and very little error handling. But it works. Though, for simplicity I made it so that an import would overwrite the entire database. With a confirmation prompt of course.

@arantius

This comment has been minimized.

Collaborator

arantius commented Dec 22, 2017

I think ideally the user could choose to replace or merge the import. Stylus does only merge, which is also bad IME. User choice, with defaults that might work, seems best.

@Sxderp

This comment has been minimized.

Contributor

Sxderp commented Dec 23, 2017

Sure, but then we run into a lot of conditionals. I'd like to see a flow diagram of some sort on the exact nature of merging.

Mostly in terms of conflicts. Overwrite content, overwrite key/values (yes those are exported too), prompt for everything (that seems like a bad user experience), etc?

@arantius

This comment has been minimized.

Collaborator

arantius commented Dec 23, 2017

I mean just: import every script in the file (and completely overwrite to that state), but don't touch/delete scripts that aren't in the file.

@Sxderp

This comment has been minimized.

Contributor

Sxderp commented Dec 23, 2017

Oh. That's far simpler. Yeah, that sounds good.

@arantius arantius changed the title from Add export function to Add backup/restore Jan 6, 2018

@arantius arantius removed this from the 4.x milestone Jan 19, 2018

@Sxderp

This comment has been minimized.

Contributor

Sxderp commented Jan 21, 2018

So I've implemented three different import methods.

  1. Merge. Do not touch any installed scripts. Only scripts that do not currently exist[1] will be added to the database.
  2. Replace. Uninstall all current scripts and replace them with the imported ones.
  3. Overwrite. Like merge but if a conflict is found[1] then the scripts being imported take precedence.

Waiting on other PRs before submitting.

[1] I determine uniqueness by a script's id.

@Eselce

This comment has been minimized.

Contributor

Eselce commented Jan 23, 2018

What about this?

  1. Update. Only import (overwrite) scripts that already exist.

This is complementary to 1. Merge

@Eselce

This comment has been minimized.

Contributor

Eselce commented Jan 23, 2018

  1. Overwrite. Like merge but if a conflict is found[1] then the scripts being imported take precedence.

@Sxderp I guess, "Like merge" does not include the constraint "Only scripts that do not currently exist"...
(this contraint does not make sense here) That means, Overwrite = Import all scripts, conflicting scripts are overwritten by archive...

@arantius

This comment has been minimized.

Collaborator

arantius commented Jan 23, 2018

Which of these (update/merge/etc.) options do VM, TM, or otherwise offer?

@Sxderp

This comment has been minimized.

Contributor

Sxderp commented Jan 23, 2018

  1. Update. Only import (overwrite) scripts that already exist.

Sure, that should be doable.

That means, Overwrite = Import all scripts, conflicting scripts are overwritten by archive...

Correct.

Which of these (update/merge/etc.) options do VM, TM, or otherwise offer?

Not actually sure. But this brought up another concern of mine. Compatibility with archives. Assuming those addons support full database export (scripts + data), then ideally the imports should be compatible with eachother.

@Sxderp

This comment has been minimized.

Contributor

Sxderp commented Jan 23, 2018

Update. Only import (overwrite) scripts that already exist.

Now that I think about this (all of like 3 minutes), I have a concern. What about associated data (get/setValue)? Should the data that's currently stored stay? Should the imported data take precedence? Some kind of merge? And then clearly representing this option / state to the user. This is actually a little more tricky than a first glance.

@Sxderp

This comment has been minimized.

Contributor

Sxderp commented Jan 23, 2018

Did some quick checks, this isn't 100% thorough but gives a general overview, as well as what I should probably change in regard to my implementation.

VM and TM both export into zip files. Inside the zip files you can find .user.js files for each of your scripts. However, as for exporting storage / implementation specific details, they both do it a little different. VM wraps implementation specific details and script data, for what looks like all scripts, into a single file, violentmonkey. TM does this a bit more sanely. Implementation specific details, for each script, are exported to .options.js files, while script data is exported to .storage.json.

I think I'm going to rework my implementation to follow TM. In general I think it's a better standard as it separates implementation details from the data.

As for 'method of import' (described in my post above):

TM provides a 'select each script' interface. When you import an archive you select if you want to import each specific script. If you check it then it'll overwrite all data for that script.

VM only seems to offer what I described above as 'Overwrite'. However, there is an option to NOT import associated script data (global for all scripts). From a UI perspective, I think implementing that option in GM would be difficult. However, if we take the TM approach to exporting / importing the database then a user could just open the archive and remove the .storage.json file.

@Sxderp

This comment has been minimized.

Contributor

Sxderp commented Jan 28, 2018

If anyone wants to test, I've updated my branch, sxderp:import-export-database-merge, to include the .zip functionality I discussed previously. I didn't implement the update option that @Eselce suggested because I'm still unsure about the particulars.

@arantius arantius added this to the Features milestone Feb 22, 2018

@ArmEagle

This comment has been minimized.

ArmEagle commented Mar 17, 2018

@Sxderp Thank you for developing this feature. I've built the package and installed it in Firefox Developer Edition (to enable installing unsigned extensions). I upgraded an existing profile with that browser version, overwrote the existing GreaseMonkey installation and exported my database without a problem.

Importing in a new Firefox profile with this GreaseMonkey installed caused an issue though. It seems that one specific userscript which is pretty large (both user.js and gm_details.js are around 230K is causing import issues. After replacing the database GM stops working. Clicking on the GM icon opens an empty dropdown (about 10x10 square).

I'd rather not share the specific userscript publicly. Using this trick I've found your email address and I'll send you a message that way.

Edit: The only way to make GM work after it breaks is to remove the extension, restart Firefox and add it again.

@Eselce

This comment has been minimized.

Contributor

Eselce commented Mar 17, 2018

GMexport_20180217_164139.zip ???

@Eselce

This comment has been minimized.

Contributor

Eselce commented Mar 17, 2018

Imported a whole bunch of (in part) large scripts (> 20) from a 3.4 MB zip file without complaint. No detailed examination though...

  return 'GMexport_'
       + date.getFullYear().toString()
       + date.getMonth().toString().padStart(2, '0')
       + date.getDate().toString().padStart(2, '0')
       + '_'
       + date.getHours().toString().padStart(2, '0')
       + date.getMinutes().toString().padStart(2, '0')
       + date.getSeconds().toString().padStart(2, '0')
       + '.zip'

IIRC, getMonth() starts at 0... ; missing...

@Sxderp

This comment has been minimized.

Contributor

Sxderp commented Mar 17, 2018

IIRC, getMonth() starts at 0... ; missing...

RIP

I haven't worked on this branch in a while. I'm going to take today and rebase on master and do a bunch of cleanup.

@Sxderp

This comment has been minimized.

Contributor

Sxderp commented Mar 17, 2018

I've rebased my branch on master and made some significant changes to the layout which I think offers better readability.
@ArmEagle On my updated branch I was able to import the file you sent. If you're still having problems let me know and I'll look into it further.

@arantius arantius modified the milestones: Features, 4.4 Mar 18, 2018

@arantius arantius closed this in 7fe8bfe Mar 28, 2018

@arantius

This comment has been minimized.

Collaborator

arantius commented Mar 28, 2018

Merged with changes in 7fe8bfe . Reopening to track/discuss some issues I've seen.

@arantius arantius reopened this Mar 28, 2018

@Sxderp

This comment has been minimized.

Contributor

Sxderp commented Mar 29, 2018

Reopening to track/discuss some issues I've seen.

What were the issues?

@arantius

This comment has been minimized.

Collaborator

arantius commented Mar 29, 2018

Was a one off quick test, I'm not sure, some amount of script I expected to be installed was not listed. But I want to repeat it and track carefully and write down the results. There's a lot of possible cases.

@arantius

This comment has been minimized.

Collaborator

arantius commented Apr 5, 2018

I've finally looked into this a bit.

It backs up only the .user.js, and the "details". Which happens to accidentally include the requires content, but misses the resources (JSON serializing the blob down to {}). AFAICT it will .setKnownResources() with empty/broken blobs, and thus fail to work.

It would seem better if every script has its own folder, that folder contains a file for the main .user.js, for each @require and @resource, and then perhaps the remaining parsed/custom details, and stored values. I think this might even be better enough that I'd forego any cross platform compatibility.

@Eselce

This comment has been minimized.

Contributor

Eselce commented Apr 5, 2018

Totally agreed. Just like saving a document "as web page", which would create a htm-document plus a folder of the same (base) name with the dependencies...

@Sxderp

This comment has been minimized.

Contributor

Sxderp commented Apr 5, 2018

Which happens to accidentally include the requires content ...

Wasn't by accident. Includes it on purpose, for any local changes that may have occurred. I'd consider the resources / requires as 'implementation details', and therefore exported as "details." TM doesn't even export resources / requires.

but misses the resources (JSON serializing the blob down to {}). AFAICT it will .setKnownResources() with empty/broken blobs, and thus fail to work.

There is a TODO to figure out if blobs stringify nicely. This feature was technically merged before I submitted a PR for it.

@Sxderp

This comment has been minimized.

Contributor

Sxderp commented Apr 6, 2018

For the time, the Blob issue would be resolved with #2937.

As for creating more files in the backup archive, I don't think it would confer all benefit. While archiving resources and requires and whatever out into separate files may look nice when extracted, doing this would likely make the code more complex as you're dealing with a lot of small things rather than a simple dump.

@arantius

This comment has been minimized.

Collaborator

arantius commented Apr 18, 2018

This is the "big feature" for 4.4 so I really hope to finish/improve this soonish.

@arantius

This comment has been minimized.

Collaborator

arantius commented May 2, 2018

Second thought, this is functional. There are some enhancements I'd like to track, but an issue each for those targeted changes will be easier to manage.

@arantius arantius closed this May 2, 2018

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