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

App Dependencies [WIP] #10777

Closed
DeepDiver1975 opened this issue Aug 31, 2014 · 34 comments
Closed

App Dependencies [WIP] #10777

DeepDiver1975 opened this issue Aug 31, 2014 · 34 comments
Assignees

Comments

@DeepDiver1975
Copy link
Member

Apps can have dependencies in order to operate properly. We distinguish between hard and soft dependencies.

Hard dependencies are an explicit requirement without such an dependency being available we shall not allow the app to be installed.
Soft dependencies can enable additional functionalists which are not required we shall notify the admin before installation about missing optional dependencies but installation shall still be possible.

In addition to list dependencies on each individual app there has to be an overview page which will list all dependencies.

App dependencies will be listed in the info.xml using following schema:

<dependencies>
    <dependency mode="hard" type="module" os="linux"
        name="imap" doc-link="https://doc.owncloud.org/....">
        <info>The PHP module 'imap' is required to access your email boxes.</info>
    </dependency>
</dependencies>

The attribute 'os' is used for specifying that this requirement is valid for. Can be either linux, win or mac

Available types:

  • module: a php module which is required by the app
  • class: a php class which is required
  • function: an php functions which is required (to be tested using OC_Helper::is_function_enabled())
  • ini: an php.ini setting
  • command: a command line tool to be available
  • php: specifying the minimal php version number
  • databases: the list of supported databases
  • os: the list of supported operating systems
@ghost
Copy link

ghost commented Aug 31, 2014

Hi,

so this should solve: #10775

@DeepDiver1975
Copy link
Member Author

@karlitschek please review as discussed

@jancborchardt mockitup - THX

@ghost
Copy link

ghost commented Aug 31, 2014

Hi,

currently the SMB external storage using smbclient as well as some previews are only supported on non-windows systems. This could be also added to this.

@DeepDiver1975
Copy link
Member Author

currently the SMB external storage using smbclient as well as some previews are only supported on non-windows systems. This could be also added to this.

THX

@ghost
Copy link

ghost commented Aug 31, 2014

The attribute 'os' is used for specifying that this requirement is valid for. Can be either linux, win or mac

I think this could be also extended to allow linux AND mac together as most dependencies which are checked are only doing a "runningOnWindows()"

@karlitschek
Copy link
Contributor

looks good 👍

@LEDfan
Copy link
Member

LEDfan commented Aug 31, 2014

Maybe it's a good idea to add an extra dependency type app, which can be hard an soft. This way the Chat app can have a soft dependency on the contacts app for extra functionality. Just like the maps app which can have a soft dependency on contacts, calendar and tasks.

@karlitschek
Copy link
Contributor

There should be no dependencies between app. So the calls to contacts or calendar should go through a specific interface in core.

@LEDfan
Copy link
Member

LEDfan commented Aug 31, 2014

Yeah indeed and that's the case for those apps. But the user/admin should be aware of that extra functionality. Maybe instead of soft/hard mode an extra optional mode should be introduced specific for this case.

@DeepDiver1975
Copy link
Member Author

Well - you do not have a dependency on Contacts - but a dependency to the public api.
Using this public api you can access ANY address books providers.

Assuming there are only address books taken from LDAP - there is no need for depending on the Contacts app.

@ghost
Copy link

ghost commented Sep 2, 2014

Hi,

two small additions:

attribute "os": Can be linux AND mac (as posted above)
attribute "cron": Needs (web)cron background job (like the news app)

@ghost
Copy link

ghost commented Sep 7, 2014

Hi,

seems there is also a older proposal available here: https://github.com/owncloud/core/wiki/spec-proposal:-Depend-Check-proposition

@Niduroki
Copy link
Member

#4017 (comment) @DeepDiver1975 changed your mind? 😄

@DeepDiver1975
Copy link
Member Author

#4017 (comment) @DeepDiver1975 changed your mind? 😄

❓ what ❓

@Niduroki
Copy link
Member

@DeepDiver1975: Citing you:

No direct inter app dependencies!

In case there is the need for two app communicating to each other a core api has to be used where one app registers a service which another app can consume.

As example have a look at the public contacts api in lib/public/contacts.php

In case you miss an api for an interaction of a certain type - please get in touch with us core devs and we will design a proper api together

What is this PR about? Direct inter-app dependencies? I already wrote some code for this issue, but you've dropped it because this is not what we want.

@DeepDiver1975
Copy link
Member Author

@Kondou-ger no no - we still don't want inter app dependencies - this issue describes how we can handle dependencies of apps to their environment - like php module imap is required and so on

@BernhardPosselt
Copy link
Contributor

I'd also include the php version since not having this at setup makes the process of checking pretty hard

@BernhardPosselt
Copy link
Contributor

Another one would be supported databases

@BernhardPosselt
Copy link
Contributor

Linking this one here to show the usecase for PHP minimum version :) owncloud-archive/news#614

@BernhardPosselt
Copy link
Contributor

Some additional things: PHP libs have no consistent way of providing their version. Some can be checked by using phpversion('libname') others provide global constants (infamous example: libxml).

Would it be possible to create a file like appinfo/preinstall.php that is included in addition to this where you can define your own checks and throw a generalized exception like LibraryDependencyException in case things dont work out?

@DeepDiver1975
Copy link
Member Author

Thanks for your input @Raydiation - much appreciated.

You are now talking about php extensions - no php libs which are loaded using composer (because composer is taking care about this version nightmare)

For php extensions I'm wondering if there is a need for this because e.g. php 5.4 would for sure have a much older e.g. libxml version shipped - right?

In this case devs might even need to compare the lib version based on the php version?

Or do I miss the point?

@BernhardPosselt
Copy link
Contributor

I'm unsure but I think you can upgrade the libxml and have a newer version in PHP available, regardless of php version

@DeepDiver1975
Copy link
Member Author

@karlitschek any objections in moving away from info.xml to info.json?

Parsing json is much more comfortable in PHP than this xml parsing.
As a result we could reduce code and build a more error prone code base.

For sure the existing apps with info.xml will continue to be supported for some time.

Let me know what you think - THX

@BernhardPosselt
Copy link
Contributor

👍 very good idea

What about calling it app.json instead? The json file includes general app configuration and metadata

@karlitschek
Copy link
Contributor

sorry but there is no real improvement in using json beside being cooler. Let´s stay with the current format please in invest the time in bugfixing and features please :-)

@DeepDiver1975
Copy link
Member Author

sorry but there is no real improvement in using json beside being cooler.

besides the fact that implementing the xml parsing for these new dependency entries there sill some hours be lost for implementation and testing - but as you command: we will stay with the format

Any please bare in mind: I will never propose any thing just because of being cooler!

@th3fallen
Copy link
Contributor

i have to 👍 The idea of transitioning to json as well. Maybe in the future though, its sooo much easier and faster to both parse and write than xml.

@BernhardPosselt
Copy link
Contributor

<dependencies>
    <dependency mode="hard" type="module" os="linux"
        name="imap" doc-link="https://doc.owncloud.org/....">
        <info>The PHP module 'imap' is required to access your email boxes.</info>
    </dependency>
</dependencies>

Why do we have the os tag and type? If we require an OS we could do

<dependencies>
    <dependency mode="hard" type="os" name="linux">
        <info>Linux is required</info>
    </dependency>
</dependencies>

@DeepDiver1975
Copy link
Member Author

The os-attribute is available to restrict an dependency to an platform. At least this was the intention - no idea if we really need that.

As an example: on MAC we require a certain php module which is not necessary of others.

@DeepDiver1975
Copy link
Member Author

note to myself - try to map this

{
    "dependencies": {
        "databases": ["pgsql", "sqlite3", "mysql"],
        "php": ">=5.4",
        "owncloud": ">=7.0.3",
        "libs": {
            "curl": "*",
            "libxml": ">=2.7.8",
            "SimpleXML": "*"
        }
    }
}

determined-challenge-accepted

@BernhardPosselt
Copy link
Contributor

Some inspiration

<php version=">=5.4">
<databases>
    <database version="*">pgsql</database>
    <database>sqlite3</database>
    <database>mysql</database>
</databases>
<libs>
    <lib version="*">curl</lib>
    <lib version=">=2.7.8">libxml</lib>
    <lib>SimpleXML</lib>
</libs>

Whenever version is not given, * is assumed

@BernhardPosselt
Copy link
Contributor

and since <require*> is not in a dependencies tag i see no need to put this into a separate tag

@DeepDiver1975
Copy link
Member Author

looks like an easy structure ... thx for the inspiration

@DeepDiver1975
Copy link
Member Author

fixed with 8.0 -> close

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

7 participants