akeeba / fof

Rapid Application Development framework for Joomla!™ 3 and 4
0 stars 0 forks source link

Trying adding alternative dependecies for the phpparser #650

Closed Skullbock closed 7 years ago

photodude commented 7 years ago

You need to regenerate the composer.lock file on your local and include that new file.

Skullbock commented 7 years ago

i updated the entire composer.lock (composer update). I hope that's all right, otherwise i'll just do composer update nikic/php-parser

photodude commented 7 years ago

Something is not quite right on php 5.3

Problem 1
    - Installation request for nikic/php-parser v3.0.2 -> satisfiable by nikic/php-parser[v3.0.2].
    - nikic/php-parser v3.0.2 requires php >=5.5 -> your PHP version (5.4.37) does not satisfy that requirement.
  Problem 2
    - Installation request for phpdocumentor/reflection-common 1.0 -> satisfiable by phpdocumentor/reflection-common[1.0].
    - phpdocumentor/reflection-common 1.0 requires php >=5.5 -> your PHP version (5.4.37) does not satisfy that requirement.
  Problem 3
    - Installation request for phpdocumentor/reflection-docblock 3.1.1 -> satisfiable by phpdocumentor/reflection-docblock[3.1.1].
    - phpdocumentor/reflection-docblock 3.1.1 requires php >=5.5 -> your PHP version (5.4.37) does not satisfy that requirement.
  Problem 4
    - Installation request for phpdocumentor/type-resolver 0.2.1 -> satisfiable by phpdocumentor/type-resolver[0.2.1].
    - phpdocumentor/type-resolver 0.2.1 requires php >=5.5 -> your PHP version (5.4.37) does not satisfy that requirement.
  Problem 5
    - phpdocumentor/reflection-docblock 3.1.1 requires php >=5.5 -> your PHP version (5.4.37) does not satisfy that requirement.
    - phpspec/prophecy v1.6.2 requires phpdocumentor/reflection-docblock ^2.0|^3.0.2 -> satisfiable by phpdocumentor/reflection-docblock[3.1.1].
    - Installation request for phpspec/prophecy v1.6.2 -> satisfiable by phpspec/prophecy[v1.6.2].
Skullbock commented 7 years ago

The first one is the one that bothers me. Why doesn't it take the 2.0 version? mmmm

photodude commented 7 years ago

It's something with the composer.lock file... looking into it

photodude commented 7 years ago

I'm not sure but Try this with double pipes and an expanded php range

{
    "name": "akeeba/fof",
    "type": "library",
    "description": "FOF (Framework on Framework) is a Rapid Application Development framework for Joomla!",
    "require": {
        "php": "~5.3.10||>=5.4,<5.5||>=5.5",
        "nikic/php-parser": "~1.0||~2.0||~3.0"
    },
    "keywords": ["framework","joomla","mvc","rad"],
    "homepage": "https://github.com/akeeba/fof",
    "license": "GPL-2.0+",
    "authors": [
        {
            "name": "Nicholas K. Dionysopoulos",
            "email": "nicholas_NO_SPAM_PLEASE@akeebabackup.com",
            "homepage": "http://www.dionysopoulos.me",
            "role": "Lead Developer"
        }
    ],
    "require-dev": {
        "mikey179/vfsStream": "dev-master",
        "phpunit/phpunit": "~4.8||~5.7",
        "phpunit/dbunit": "~1.4||~2.0",
        "mockery/mockery": "0.9.4||~0.9.5",
        "fiunchinho/phpunit-randomizer": "1.0.*@dev"
    },
    "autoload" : {
        "psr-4": {
            "FOF30\\": "fof"
        }
    }
}
photodude commented 7 years ago

The json above works (it's a bit ugly, but it works)

I'm not sure if it will work with the lock file, so I'm looking at alternatives

Skullbock commented 7 years ago

Ok, I'll try tomorrow as soon as I get back at the office thanks! Il giorno gio 2 feb 2017 alle 20:01 Walt Sorensen notifications@github.com ha scritto:

The json above works (it's a bit ugly, but it works)

I'm not sure if it will work with the lock file, so I'm looking at alternatives

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/akeeba/fof/pull/650#issuecomment-277049783, or mute the thread https://github.com/notifications/unsubscribe-auth/ABDY01xcZqvbXgb09nuF2XuKLuwI69ofks5rYigKgaJpZM4L1JU1 .

-- Daniele Rosario CTO Via F.Filzi, 56/A 36051 Creazzo (Vicenza) Tel.: +39 0444 1454934 Mob: +39 328 3017134 Immagine in linea 1

photodude commented 7 years ago

Seems something is wrong with the other parts of the lock file now (from the previous lock file update at 6bf6c85 )

photodude commented 7 years ago

I did a test without the lock file https://travis-ci.org/photodude/fof/builds/197738151 and everything works, so just need to sort out the lock file issues.

Maybe try deleting the lock file and create a new one.

Skullbock commented 7 years ago

Tried it, let's see :)

Skullbock commented 7 years ago

This is sooo strange...

photodude commented 7 years ago

I'm guessing the composer lock file is somehow dependent on the php version it is created on, and you created it on php 5.6 or php 7. I have no idea if there is a way to generate a generic lock file that works for the different version ranges.

Skullbock commented 7 years ago

I could generate it on php56 so travis is happy, let me try

Skullbock commented 7 years ago

This is generated on 5.4 and seems to read the correct version...

Skullbock commented 7 years ago

fails anyway, should we put it into require-dev and let it go?

photodude commented 7 years ago

Looks like there may be only two solutions, (which I think is bad on either side)

Considering we see this as a dev-only tool, it probably belongs in the require-dev section anyway (although I think require-dev means required for developing the package not required for developers)

Skullbock commented 7 years ago

Yeah it would mean that, but who'll use this tool will probably be an advanced user so...

photodude commented 7 years ago

Moving it to require-dev solves portability for consuming packages, but does not solve the travis CI testing issues as that runs in the require-dev mode.

Skullbock commented 7 years ago

I really don't know what to say then... Not that much of a travis expert unfortunately

photodude commented 7 years ago

It's a composer issue, not travis (All I know about composer is what I have read and have seen others do).

Skullbock commented 7 years ago

What if we composer update in the travis job? That way the composer.lock file would be regenerated by travis on the fly, respecting the dependencies?

photodude commented 7 years ago

maybe that could work, I would delete the lock file in travis before the composer update. Like I said, probably a bit of a hackish solution but sometimes we have no other option.

Skullbock commented 7 years ago

@nikosdion @tampe125 What do you guys think?

nikosdion commented 7 years ago

None of these approaches make me happy :/

Removing the lock file is bad because each developer working on this repository will end up working against different versions of Composer packages.

require-dev is only to be used for development. This is not the case, you need that code for the tool.

That said, adding the requirements into the mail composer.json is equally bad. Keep in mind that the CLI tool is to be packaged as a PHAR archive. If your vendor folder is outside the tool's structure, how is it going to be packaged in it?.

It seems to me that the best temporary solution is to create a new composer.json inside the tool's directory. This is an architecturally correct solution, too! The CLI tool is independent of the main repo and vice versa. So I'd say do this instead.

Finally, the permanent solution is to, of course, move the tool to a new repository but that's something I am not willing to do right now :)

Skullbock commented 7 years ago

ok, i'll try to do that. How should i then load both composer.json in the cli tool?

nikosdion commented 7 years ago

FOF is one thing. The CLI script is another thing. Two separate composer.json files for two separate things. One does not load the other.

Skullbock commented 7 years ago

I meant that in the fof cli tool we use the fof generators, factories, etc. Can i just put fof as a composer dependency and that's it?

nikosdion commented 7 years ago

No. The CLI script needs to be configured with the site's root. Then it loads FOF from the site's libraries/fof30 directory. You don't need to add FOF as a Composer dependency. FOF is something you must install on the site before using this tool.

Skullbock commented 7 years ago

Ok, so the only dependencies i need to specify are the tool's one, and those have to be removed from the fof library. Ok