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

[Feature Request] make the apps-external folder default used in new installations #29839

Closed
mmattel opened this issue Dec 13, 2017 · 11 comments · Fixed by #34898
Closed

[Feature Request] make the apps-external folder default used in new installations #29839

mmattel opened this issue Dec 13, 2017 · 11 comments · Fixed by #34898

Comments

@mmattel
Copy link
Contributor

mmattel commented Dec 13, 2017

References:
#29807 (Move apps from apps to apps2 folder can cause issues)
https://github.com/owncloud/documentation/issues/3621 (Describe procedure how to move downloaded and installed apps from marketplace to the apps2 folder)

Background:
When you have a new install, config.php gets created and it is important to have all the settings present respectively created early which may affect you when you go for upgrades later on. App management for apps not coming from core is one of those things.

When upgrading an installation via tar, it is a clean way to rename the old instance and extract into an empty folder. Copy your config.php and lucky when aliased your data directory. After doing all the steps to finalize, you just need to do an ./occ upgrade where core does the rest.
But what you have to do manually, identify all those apps which are in the old instance and copy them to the apps folder of the new instance and hopefully do not mix up things... This step is not necessary if the proposal below is implemented.

Proposal:
Adding as default into the created config.php the ability to use the apps-external folder.
see: https://doc.owncloud.org/server/latest/admin_manual/installation/apps_management_installation.html?highlight=apps2

  "apps_paths" => array (
      0 => array (
              "path"     => OC::$SERVERROOT."/apps",
              "url"      => "/apps",
              "writable" => false,
      ),
      1 => array (
              "path"     => OC::$SERVERROOT."/apps-external",
              "url"      => "/apps-external",
              "writable" => true,
      ),
  ),

Benefit:
You do not need to take care on moving/copying apps anymore.
You only need to either copy or alias the apps2 folder to the correct location.

@PVince81 @settermjd

@mmattel
Copy link
Contributor Author

mmattel commented Mar 21, 2018

To be added at:

$config->setSystemValue('installed', true);

@cdamken
Copy link
Contributor

cdamken commented Mar 22, 2018

we have to add it in the config.sample.php for documentation

@mmattel
Copy link
Contributor Author

mmattel commented Mar 22, 2018

@cdamken pls see issue owncloud-archive/documentation#3933 (config.sample.php issues)
Your point is not the only one I identified

@mmattel
Copy link
Contributor Author

mmattel commented Mar 22, 2018

@cdamken see PR: owncloud-archive/documentation#3934 to fix the config.sample.php issue

@PVince81
Copy link
Contributor

Have you tried creating a separate file before running setup:

  • create "config/apps2.config.php" containing the settings to bootstrap
  • run occ maintenance:install

It seems your purpose of "apps2" is to have a separate read-write folder. Maybe it could be called "apps-updated" or something that reflects its purpose.

From what I remember some docker based deployments also have two apps folders, one read-only as provided by the tarball and one for the updates.

cc @VicDeo

@mmattel
Copy link
Contributor Author

mmattel commented Mar 23, 2018

@PVince81

Acording to: https://doc.owncloud.org/server/latest/admin_manual/installation/apps_management_installation.html?highlight=apps2
Using Custom App Directories

  1. It separates ownCloud’s core apps from user or admin downloaded apps. Doing so distinguishes which apps are core and which aren’t, simplifying upgrades.
    ...
    I took the naming as it was originally in the documentation (since ages).

The idea to have this seperated is really great as you know what is delivered from core aka basic install and what is manually downloaded. This is especially true for upgrades - see the issue text above.

@cdamken
Copy link
Contributor

cdamken commented Mar 26, 2018

@mmattel

@cdamken see PR: owncloud-archive/documentation#3934 to fix the config.sample.php issue

Thanks

From what I remember some docker based deployments also have two apps folders, one read-only as provided by the tarball and one for the updates.

@PVince81What will happen with the market app updater? or while an upgrade from Tar-ball that runs the market app?

@mmattel
Copy link
Contributor Author

mmattel commented Mar 26, 2018

Fyi, I am running a config with apps / apps2 successfully for about 4 months.
Did all the owncloud upgrades via tar and upgrades for custom downloaded apps with the market updater without any problems / issues.

@mmattel mmattel changed the title [Feature Request] make the apps2 folder default used in new installations [Feature Request] make the apps-external folder default used in new installations Apr 11, 2018
@PVince81
Copy link
Contributor

as discussed with @patrickjahns:

  • we'd like to not bundle the folder with tarballs as it would make the folder appear even on updates, not only new installations. This means that admins that already have a setup with a different separate apps folder will see this new folder appear and be left wondering.
  • also, most other folders are created at install time like "data"
  • create "apps-external" at install time only and make it writeable
  • revert makefile changes from [stable10] Add per default the apps-external directory in config.php during installation #34656

@mmattel does that make sense ?

@PVince81
Copy link
Contributor

@mmattel would you be able to make the required changes ?

@mmattel
Copy link
Contributor Author

mmattel commented Mar 26, 2019

Affects master #30889 and stable10 #34656, both merged.
Can be done, just reading the code.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants