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] improvement for update location for shipped apps #30904

Closed
mmattel opened this issue Mar 26, 2018 · 9 comments
Closed

[Feature Request] improvement for update location for shipped apps #30904

mmattel opened this issue Mar 26, 2018 · 9 comments

Comments

@mmattel
Copy link
Contributor

mmattel commented Mar 26, 2018

Referencing:
Issue: #29839 ([Feature Request] make the apps-external folder default used in new installations)
PR: #30889 (Add per default the apps-external directory in config.php during installation)

Prerequisit:
You install ownCloud where the key apps_path gets added by default or you have manually added the key (plus moving manually added apps to the apps-external directory). Important: apps/ is marked readonly and apps-external is writeable.

In case yo do not use the apps_path key (means apps/ is writeable) it does not matter !

Situation:

  1. You update a user downloaded app. Because the app is in apps-external, and the directory is set to write, the update automatically goes to this directory.
  2. Because of an "emergency" situation, a core shipped app needs an update. This update would now go to apps-external too. Remember apps is read only.
  3. In a regular situation, a new version of a core app is automatically delivered via the tarball and is by design in apps.

The beauty of having apps and apps-external is the extremly ease of owncloud upgrades with respect of apps file handling and keeping a good overview respectively easy distinguish between what is shipped with core and what is downloaded manually.

In case of 2.) an update of a core app would now go into apps-external which is not a problem but uglifies what was beauty before. You have now a mix between core and manually downloaded apps in apps-external and carry this (remember only because it was an emergency update of a shipped app which will be part of the regular owncloud update anyways and part of apps) forever except you manually delete it after an owncloud upgrade.

Feature Request:
Add a functionality, maybe enfored with a key like coreapps.update.location.original => true- tbd, that core shipped apps are always written to their original location, not "polluting/uglyfying" apps-external. Again no change if there is only apps.

@PVince81 @cdamken

@ownclouders
Copy link
Contributor

GitMate.io thinks the contributor most likely able to help you is @PVince81.

Possibly related issues are #8569 (Feature requests), #12530 (Feature Request), #21366 (Update), #20661 (Update), and #6250 (New feature request).

@mmattel
Copy link
Contributor Author

mmattel commented Mar 29, 2018

Doing some research, a question raises.

The code where you would need to do changes is in /core/lib/private/Installer.php#L224

public static function updateApp($info= [], $isShipped=false) {
	list($extractDir, $path) = self::downloadApp($info);
	$info = self::checkAppsIntegrity($info, $extractDir, $path, $isShipped);

self::checkAppsIntegrity

...
$info = OC_App::getAppInfo($extractDir.'/appinfo/info.xml', true);
...
// check if shipped tag is set which is only allowed for apps that are shipped with ownCloud
if(!$isShipped && isset($info['shipped']) && ($info['shipped']=='true')) {
	OC_Helper::rmdirr($extractDir);
	throw new \Exception($l->t("App can't be installed because it contains ...
}
  • For me this means, that a shipped app can never be upgraded via an automated process independent of an ownCloud upgrade.
  • Following that there is NEVER an emergency upgrade of a single shipped app possible.
  • Emergency upgrades of shipped apps are only in conjunction with an owncloud upgrade because that is the only way they are delivered.

My question: true or false ??

If true: this issue can be closed and this behaviour should be documented.

@PVince81 @VicDeo

@VicDeo
Copy link
Member

VicDeo commented Mar 29, 2018

@mmattel shipped is a legacy appinfo.xml tag. Currently the list of shipped apps is maintained in a separate file https://github.com/owncloud/core/blob/master/core/shipped.json
So if the app has shipped tag and is not included into https://github.com/owncloud/core/blob/master/core/shipped.json this means it hasn't been updated for a long time.

@mmattel
Copy link
Contributor Author

mmattel commented Mar 29, 2018

@VicDeo thanks, I have seen that there is no shipped tag in info.xml anymore.
But from the code, as far as I understood, the app name is queried against shipped.json and a match is returned as shipped or not, identifying if the app is part of core delivery.

Based on that, I assume my statements above - core apps (eg as part of the tar ball) are never updated independent of a new owncloud release. This is what I wanted to clarify by my finding when going thru the code.

A result of this finding can be a documentation improvement:

  • Apps delivered with the owncloud package (eg tar) are never updated independently.
  • Or can be updated independently but then the question raises how as these apps are not visible at the marketpace on their own.

@mmattel
Copy link
Contributor Author

mmattel commented Apr 11, 2018

OK, my assumption that core apps are never independently updated is definitely wrong.

Statement: core apps can be independently updated

The reason for this is easy:
Today I got an app upgrade alert telling me that the "Mail Template Editor" can be upgraded from templateeditor: 0.2 to templateeditor: 0.3. This app is a core shipped app.

This means, that my initial driver for this issue is valid and the proposal to treat core shipped apps differently with respect to the upgrade location is correct.

@PVince81 @patrickjahns @tboerger
This is maybe of interest to you. Even we currently do not have an automatism to have the apps-external folder shipped, we have the config key and admins use it - including our docker setup! I highly suggest to have a mechanism to enable upgrading shipped apps to their original location in case the apps_path key is used.

@VicDeo
Copy link
Member

VicDeo commented Apr 11, 2018

@mmattel templateeditor is no longer shipped since 10.0.8 ;)
A marketplace release is a part of unbundling the app
https://marketplace.owncloud.com/apps/templateeditor

@mmattel
Copy link
Contributor Author

mmattel commented Apr 11, 2018

Thanks @VicDeo .
Closing

@mmattel mmattel closed this as completed Apr 11, 2018
@tboerger
Copy link
Contributor

Within the docker image the updated app gets installed into the persistent apps folder, the bundled app is not used in that case anymore.

@mmattel
Copy link
Contributor Author

mmattel commented Apr 19, 2018

Just for completeness.
The market app just upgraded. It is a shipped app.
Checking apps and apps-external folder before upgrading and after.
The market app upgraded into apps and not into apps-external folder.
So everything is ok.

@lock
Copy link

lock bot commented Jul 30, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jul 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants