Desktop: Resolves #7934: Add Simple Backup as a default plugin #9360

personalizedrefrigerator · 2023-11-23T03:33:51Z

Summary

Adds 7zip-bin as a library that can be required by plugins
Adds the Simple Backup plugin as a submodule
- This may require extra steps after cloning. To-do: Can this be done automatically? (git submodule init && git submodule update)
- Patches the plugin to update to the latest build script (used Webpack 4 previously, which seems to require Node's --openssl-legacy-provider flag) and include 7zip-bin as an external dependency.
  - Webpack 5 also includes support for externalsType which allows keeping a require for 7zip-bin, even though the built plugin script doesn't use exports.... to export functions.
  - With Webpack 4, 7zip-bin is required with eval('require("7zip-bin")') to preserve the require statement.
- Builds the plugin on postinstall

To-do

This pull request

Fix: Automatically pull submodules
Fix: On a first postinstall, the default plugin build script tries to use execCommand from @joplin/utils before @joplin/utils has been built.
Fix: The patch file for Simple Backup is currently very, very large
- This is because it upgrades Simple Backup to the latest published build script, which uses Webpack 5.
- I haven't managed to correctly mark 7zip-bin as an external dependency in Webpack 4.
Refactoring: default-plugins currently writes to a directory in a different package (packages/app-desktop/build/) on build. For maintainability, it may make sense to make default-plugins a dependency of app-desktop. app-desktop can then trigger the build and provide an output path.
Upstream changes: Open the migration to the latest build script as a pull request.
- Would significantly shrink the size of the patch being applied to the plugin.
- See Update to latest plugin API JackGruber/joplin-plugin-backup#60
Testing: Verify that MacOS and Windows apps can be built (switches from 7zip-bin packages for individual platforms to the 7zip-bin package for more platforms).

Follow-up pull request

Feature: Show indication that Simple Backup is a default plugin (currently doesn't show "recommended" badge either)
Feature: Don't allow users to uninstall default plugins, but do allow rolling back to the version shipped with Joplin or disabling. To-do: What about resetting/clearing settings?
- This will also require adding a way to switch from a default plugin to a version published on NPM. (Currently an "Update" button is shown next to the plugin).
Fix: Default plugins should auto-update when installing a new version of Joplin.

personalizedrefrigerator · 2023-11-23T03:39:21Z

packages/generator-joplin/generators/app/templates/webpack.config.js

+	// We support requiring 7zip-bin through require('7zip-bin')
+	externalsType: 'commonjs',
+	externals: {
+		'7zip-bin': { commonjs: '7zip-bin' },
+	},


Suggested change

// We support requiring 7zip-bin through require('7zip-bin')

externalsType: 'commonjs',

externals: {

'7zip-bin': { commonjs: '7zip-bin' },

},

We may not want to include this for all plugins.

personalizedrefrigerator · 2023-11-23T03:48:15Z

packages/default-plugins/build.ts

+import { dirname, join, resolve, basename } from 'path';
+import { tmpdir } from 'os';
+import { chdir, cwd } from 'process';
+import { execCommand } from '@joplin/utils';


To-do: Using @joplin/utils during the first build fails if @joplin/utils hasn't been built yet.

Is it added to package.json?

Yes — however, the default plugins build script is currently run during the build process, which seems to sometimes happen before tsc compiles @joplin/utils.

Edit 1: --topological is being passed to yarn workspaces foreach, so it's possible that I've added @joplin/utils incorrectly.

Edit 2: It may be because @joplin/utils lacks a build command in its package.json. Adding "build": "yarn run tsc" seems to fix the issue (though I've only tested with one full build).

packages/default-plugins/plugin-patches/io.github.jackgruber.backup.diff

yarn.lock

…p diff

Wladefant · 2023-11-24T08:47:55Z

Absolutely, many plugins should become defaults in Joplin.

Anchor Link Plugin: This plugin is crucial but requires modification due to its current unconventional functionality, and strange behaviour in the rich editor mode.
Outline Plugin: This is definitely a necessary addition and should be a default feature.
Quick Links Feature: The implementation of '[[ ]]' instead of '@@' is recommended for this feature. It aligns with the standard practices in productivity tools and is a key aspect for user adoption.
Note Tabs: Almost ready for use as is. This feature, which has been successful in Obsidian, should also be implemented for mobile users in Joplin.
Rich Markdown :
definitely the different ctrl behaviour:
- Ctrl+Click on Images: A definite inclusion for improved usability.
- Checkboxes: Essential
- Improved Link Handling:
Favorites: Implementing a favorites system would be beneficial for user efficiency and workflow organization.

@personalizedrefrigerator if you could look at this repo SeptemberHX/joplin-plugin-enhancement#52 and integrate some of it into joplin that would be very good I think, but now I will stop spamming

laurent22 · 2023-11-24T13:48:32Z

Some issue on CI:

2023-11-24T04:12:57.8223869Z ➤ YN0000: │ root@workspace:. STDOUT ➤ YN0000: [@joplin/default-plugins]: Run `npm audit` for details.
2023-11-24T04:12:57.8480976Z ➤ YN0000: │ root@workspace:. STDOUT ➤ YN0000: [@joplin/default-plugins]: Copying published file.
2023-11-24T04:12:57.8622267Z ➤ YN0000: │ root@workspace:. STDOUT ➤ YN0000: [@joplin/default-plugins]: Error: No published files found in C:\Users\RUNNER~1\AppData\Local\Temp\default-plugin-build5F1QVf\publish
2023-11-24T04:12:57.8626997Z ➤ YN0000: │ root@workspace:. STDOUT ➤ YN0000: [@joplin/default-plugins]:     at D:\a\joplin\joplin\packages\default-plugins\build.ts:97:11
2023-11-24T04:12:57.8630502Z ➤ YN0000: │ root@workspace:. STDOUT ➤ YN0000: [@joplin/default-plugins]:     at Generator.next ()
2023-11-24T04:12:57.8637123Z ➤ YN0000: │ root@workspace:. STDOUT ➤ YN0000: [@joplin/default-plugins]:     at fulfilled (D:\a\joplin\joplin\packages\default-plugins\build.ts:6:58)
2023-11-24T04:12:57.8641147Z ➤ YN0000: │ root@workspace:. STDOUT ➤ YN0000: [@joplin/default-plugins]: Build directory C:\Users\RUNNER~1\AppData\Local\Temp\default-plugin-build5F1QVf
2023-11-24T04:12:57.8645061Z ➤ YN0000: │ root@workspace:. STDOUT ➤ YN0000: [@joplin/default-plugins]: Input is not from a TTY -- not waiting for input.
2023-11-24T04:13:08.9519275Z ➤ YN0000: │ root@workspace:. STDOUT ➤ YN0000: [@joplin/default-plugins]: Removed build directory
2023-11-24T04:13:08.9531472Z ➤ YN0000: │ root@workspace:. STDOUT ➤ YN0000: [@joplin/default-plugins]: bin.js build
2023-11-24T04:13:08.9534446Z ➤ YN0000: │ root@workspace:. STDOUT ➤ YN0000: [@joplin/default-plugins]:
2023-11-24T04:13:08.9537166Z ➤ YN0000: │ root@workspace:. STDOUT ➤ YN0000: [@joplin/default-plugins]: build all
2023-11-24T04:13:08.9539757Z ➤ YN0000: │ root@workspace:. STDOUT ➤ YN0000: [@joplin/default-plugins]:
2023-11-24T04:13:08.9542364Z ➤ YN0000: │ root@workspace:. STDOUT ➤ YN0000: [@joplin/default-plugins]: Options:
2023-11-24T04:13:08.9545566Z ➤ YN0000: │ root@workspace:. STDOUT ➤ YN0000: [@joplin/default-plugins]:   --version  Show version number                                       [boolean]
2023-11-24T04:13:08.9548887Z ➤ YN0000: │ root@workspace:. STDOUT ➤ YN0000: [@joplin/default-plugins]:   --help     Show help                                                 [boolean]
2023-11-24T04:13:08.9551604Z ➤ YN0000: │ root@workspace:. STDOUT ➤ YN0000: [@joplin/default-plugins]:
2023-11-24T04:13:08.9554904Z ➤ YN0000: │ root@workspace:. STDOUT ➤ YN0000: [@joplin/default-plugins]: Error: No published files found in C:\Users\RUNNER~1\AppData\Local\Temp\default-plugin-build5F1QVf\publish
2023-11-24T04:13:08.9558980Z ➤ YN0000: │ root@workspace:. STDOUT ➤ YN0000: [@joplin/default-plugins]:     at D:\a\joplin\joplin\packages\default-plugins\build.ts:97:11
2023-11-24T04:13:08.9562185Z ➤ YN0000: │ root@workspace:. STDOUT ➤ YN0000: [@joplin/default-plugins]:     at Generator.next ()
2023-11-24T04:13:08.9565567Z ➤ YN0000: │ root@workspace:. STDOUT ➤ YN0000: [@joplin/default-plugins]:     at fulfilled (D:\a\joplin\joplin\packages\default-plugins\build.ts:6:58)
2023-11-24T04:13:08.9851468Z ➤ YN0000: │ root@workspace:. STDOUT ➤ YN0000: [@joplin/default-plugins]: Process exited (exit code 1), completed in 3m 33s

personalizedrefrigerator · 2023-11-24T14:08:50Z

Some issue on CI:

This may be related to glob and Windows path separators:

Note Glob patterns should always use / as a path separator, even on Windows systems, as \ is used to escape glob characters. If you wish to use \ as a path separator instead of using it as an escape character on Windows platforms, you may set windowsPathsNoEscape:true in the options. In this mode, special glob characters cannot be escaped, making it impossible to match a literal * ? and so on in filenames.
¹

https://github.com/isaacs/node-glob ↩

personalizedrefrigerator · 2023-11-24T14:39:37Z

We may also need to change the default location for backups. In the dev branch, it is overridden with the profile directory:

joplin/packages/lib/services/plugins/defaultPlugins/desktopDefaultPluginsInfo.ts

Line 9 in 3b15834

'path': `${Setting.value('profileDir')}`,

However, it seems that the Simple Backup plugin can clear the backup directory (and thus the profile directory) in some cases:

https://github.com/JackGruber/joplin-plugin-backup/blob/07fafb31fe7643b5b821a5befd68be28131e0079/src/Backup.ts#L1079

Edit: Not an issue as Simple Backup by default creates a subfolder to store backups. This would only be an issue if subfolder creation was not enabled.

https://github.com/JackGruber/joplin-plugin-backup/blob/07fafb31fe7643b5b821a5befd68be28131e0079/src/Backup.ts#L266

https://github.com/JackGruber/joplin-plugin-backup/blob/07fafb31fe7643b5b821a5befd68be28131e0079/src/Backup.ts#L565

Additional issues (though possibly not actually issues):

If the backup format isn't set to JEX and two notebooks in the same parent have the same name (or only vary by <>/\ or similar characters), only one of the two notebooks will be backed up (line, line)
- May not be an issue, depends on behavior of joplin.commands.execute("exportFolders", ...)
Password inputs in settings have a default value (should perhaps be a placeholder?)
- May require changes to the plugin API.

personalizedrefrigerator · 2023-11-26T16:39:32Z

The CI is failing — `export NODE_OPTIONS="--openssl-legacy-provider" was added to support building with Webpack 4. However, Windows has different syntax for setting environment variables.

JackGruber · 2023-11-27T16:09:40Z

However, it seems that the Simple Backup plugin can clear the backup directory (and thus the profile directory) in some cases:

https://github.com/JackGruber/joplin-plugin-backup/blob/07fafb31fe7643b5b821a5befd68be28131e0079/src/Backup.ts#L1079

Edit: Not an issue as Simple Backup by default creates a subfolder to store backups. This would only be an issue if subfolder creation was not enabled.

Yes thats true, but when you uncheck Create Subfolder and point the Path to the Joplin profile directory the path should be reseted to None for the backup run and you should see an error.
https://github.com/JackGruber/joplin-plugin-backup/blob/07fafb31fe7643b5b821a5befd68be28131e0079/src/Backup.ts#L290-L292
Not sure if this prevents all cases where someone configures the path to the Joplin directory.
But should also work with C:\Users\user\.config\joplindev-desktop\JoplinBackup\...
Perhaps the error message should be improved.

If the backup format isn't set to JEX and two notebooks in the same parent have the same name (or only vary by <>/\ or similar characters), only one of the two notebooks will be backed up

I created a issue JackGruber/joplin-plugin-backup#61

Password inputs in settings have a default value (should perhaps be a placeholder?)

Is a place holder, both passwords are different :)

…uild on desktop app build)

…for easier debugging

personalizedrefrigerator · 2023-12-05T16:08:34Z

Before this PR can be merged

Verify that the default plugin doesn't overwrite an existing installation of Simple Backup
Verify that "Update" is hidden when using the default/built-in plugin.
Build the MacOS and Windows releases, compare sizes before and after.
- 7zip-bin may include unnecessary 7Zip binaries (e.g. Windows 7Zip on MacOS).
- Windows: Size after: 241 MB. Size before: 234 MB.
  - Simple Backup only needs the path to a 7zip binary. Joplin has existing dependencies on 7Zip binaries (different packages from 7zip-bin), so it should be possible to access 7Zip in a different way.
  - I'm currently working around this issue by copying just the needed 7zip binary to the build/ directory (and importing it from there). This seems to work. (Tested on MacOS, Linux, and also, before several recent changes, Windows).
    - On MacOS, this brings the .dmg file down from 208.8 MB -> 206 MB. A DMG file built from the dev branch has a size of roughly 205 MB.

We may also want to:

Decide whether we want to mark 7zip-bin as importable in the default webpack.config.js for plugins. (Do we want to allow other plugins to import 7zip-bin?)
- This pull request currently does 7zip-bin as not importable in the template.
- If we do mark it as importable, it may eventually not be necessary to patch Simple Backup.

.gitmodules

packages/default-plugins/package.json

packages/app-desktop/gui/ConfigScreen/controls/plugins/PluginBox.tsx

packages/default-plugins/package.json

laurent22 · 2023-12-06T11:52:15Z

packages/default-plugins/package.json

+{
+	"name": "@joplin/default-plugins",
+	"version": "2.13.0",
+	"description": "Default plugins bundeler",


packages/default-plugins/plugin-patches/io.github.jackgruber.backup.diff

…hors access to 7zip-bin

…caused can be ignored

laurent22 · 2023-12-11T13:58:38Z

Ok I think we can merge now, and we'll handle update in a follow up pull request. Thanks for implementing this!

personalizedrefrigerator added 5 commits November 22, 2023 11:15

WIP: Build default plugins in CI

e6a70ea

Plugin base repo

3ff8dae

Bundle note tabs and simple backup with the app

9be4526

Remove note tabs from default plugins

af9efa8

Change patch method for simple-backup

4514e1b

personalizedrefrigerator linked an issue Nov 23, 2023 that may be closed by this pull request

Include certain default plugins with the application #7934

Closed

4 tasks

personalizedrefrigerator commented Nov 23, 2023

View reviewed changes

packages/default-plugins/plugin-patches/io.github.jackgruber.backup.diff Show resolved Hide resolved

personalizedrefrigerator commented Nov 23, 2023

View reviewed changes

yarn.lock Outdated Show resolved Hide resolved

personalizedrefrigerator added 6 commits November 23, 2023 07:22

Change @joplin/utils dependency version specifier

6181e22

Try to fix @joplin/utils dependency

610b0e3

Fix build

fd88f12

Initialize submodules on build if not yet initialized

7396698

Fix "no commit author" in CI and try to decrease size of simple-backu…

9887ed7

…p diff

Use same version of @types/yargs as other packages

812fd2e

personalizedrefrigerator mentioned this pull request Nov 24, 2023

wow just wow SeptemberHX/joplin-plugin-enhancement#52

Open

personalizedrefrigerator changed the title ~~Desktop: Add Simple Backup as a default plugin~~ Desktop: Resolves #7934: Add Simple Backup as a default plugin Nov 24, 2023

Try to fix build on Windows

9db257b

personalizedrefrigerator mentioned this pull request Nov 24, 2023

Feature request: Allow building a version without 7Zip JackGruber/joplin-plugin-backup#56

Closed

personalizedrefrigerator added 4 commits November 24, 2023 12:57

Mark plugins as built-in in manifest

3cbd360

Improve plugin patch command

632f011

Fix 7zip backups

8899d8f

Significantly decrease patch size

f0545d0

personalizedrefrigerator added 9 commits November 28, 2023 20:14

Merge remote-tracking branch 'upstream/dev' into pr/default-plugins

ddfd3ec

Update @types/yargs to match recent Renovate commit

6a4cf2f

Refactor build script into multiple files

ebfc8ab

Refactor: Make default-plugins a dependency of the desktop app (and b…

3f4f1f5

…uild on desktop app build)

Remove built gulpfile

c743006

tsconfig.json: Avoid compiling gulpfile.ts

840d2e8

Update default_plugins.md

5868af5

Update simple backup plugin

2ca5a7b

Update gulpfile: Build default plugins after other portions of build …

58ffb86

…for easier debugging

laurent22 reviewed Dec 6, 2023

View reviewed changes

packages/default-plugins/plugin-patches/io.github.jackgruber.backup.diff Show resolved Hide resolved

personalizedrefrigerator added 2 commits December 6, 2023 08:27

Update setupNewRelease to also increase default-plugins package version

ad1cb0d

Spelling: Remove extra "e"

17fa5f7

personalizedrefrigerator mentioned this pull request Dec 6, 2023

Chore: Release script: Fix editor package version not updated #9458

Merged

personalizedrefrigerator and others added 13 commits December 6, 2023 18:47

Merge remote-tracking branch 'upstream/dev' into pr/default-plugins

304db99

Revert changes to webpack.config.js: By default don't give plugin aut…

40235f7

…hors access to 7zip-bin

Include only the 7zip binary for the current platform

c47c2d1

Copy 7za for the target platform and architecture, not the current

2bf0e2c

Logging

8509fa8

Add simple default plugins test

d41d3f2

Remove generated .js file from version control

4165edd

Add additional simple backup test

806c7e5

Fix importing 7zip-bin, fix "read only file system" error

4954c61

Fix importing 7zip-bin in development mode

9c581a1

Trying to make end-to-end test more reliable

efc9dc4

Make end-to-end tests more reliable -- wait for plugins to load

c1ccafe

Revert part of patch change to Simple Backup -- the error message it …

6926af8

…caused can be ignored

laurent22 merged commit 4fc786c into laurent22:dev Dec 11, 2023
10 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Desktop: Resolves #7934: Add Simple Backup as a default plugin #9360

Desktop: Resolves #7934: Add Simple Backup as a default plugin #9360

personalizedrefrigerator commented Nov 23, 2023 •

edited

Loading

personalizedrefrigerator Nov 23, 2023

personalizedrefrigerator Nov 23, 2023

laurent22 Nov 23, 2023

personalizedrefrigerator Nov 23, 2023 •

edited

Loading

Wladefant commented Nov 24, 2023 •

edited

Loading

laurent22 commented Nov 24, 2023

personalizedrefrigerator commented Nov 24, 2023

personalizedrefrigerator commented Nov 24, 2023 •

edited

Loading

personalizedrefrigerator commented Nov 26, 2023

JackGruber commented Nov 27, 2023

personalizedrefrigerator commented Dec 5, 2023 •

edited

Loading

laurent22 Dec 6, 2023

laurent22 commented Dec 11, 2023

Desktop: Resolves #7934: Add Simple Backup as a default plugin #9360

Desktop: Resolves #7934: Add Simple Backup as a default plugin #9360

Conversation

personalizedrefrigerator commented Nov 23, 2023 • edited Loading

Summary

To-do

This pull request

Follow-up pull request

personalizedrefrigerator Nov 23, 2023

Choose a reason for hiding this comment

personalizedrefrigerator Nov 23, 2023

Choose a reason for hiding this comment

laurent22 Nov 23, 2023

Choose a reason for hiding this comment

personalizedrefrigerator Nov 23, 2023 • edited Loading

Choose a reason for hiding this comment

Wladefant commented Nov 24, 2023 • edited Loading

laurent22 commented Nov 24, 2023

personalizedrefrigerator commented Nov 24, 2023

Footnotes

personalizedrefrigerator commented Nov 24, 2023 • edited Loading

personalizedrefrigerator commented Nov 26, 2023

JackGruber commented Nov 27, 2023

personalizedrefrigerator commented Dec 5, 2023 • edited Loading

Before this PR can be merged

laurent22 Dec 6, 2023

Choose a reason for hiding this comment

laurent22 commented Dec 11, 2023

personalizedrefrigerator commented Nov 23, 2023 •

edited

Loading

personalizedrefrigerator Nov 23, 2023 •

edited

Loading

Wladefant commented Nov 24, 2023 •

edited

Loading

personalizedrefrigerator commented Nov 24, 2023 •

edited

Loading

personalizedrefrigerator commented Dec 5, 2023 •

edited

Loading