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

Is there something extra needed for autoload of 3rd party functions? #352

Closed
TomasVotruba opened this issue Jan 23, 2019 · 20 comments

Comments

Projects
None yet
2 participants
@TomasVotruba
Copy link

commented Jan 23, 2019

Hi, I'm giving box + php-scoper try after few months. There have been many realeases since, great job!

I have a problem, when I load 3rd party function and run rector.phar:

Call to undefined function _HumbugBox310e0cf3f4f2\Safe\getcwd()  

Autoloading is working, this function is loaded in static class method, that was autolaoded.

composer.json (simplified)

{
    "required": {
        "thecodingmachine/safe": "^0.1.11"
    }
}

box.json

{
    "alias": "rector.phar",
    "banner": false,
    "base-path": "tmp/build",
    "check-requirements": false,
    "compactors": [
        "KevinGH\\Box\\Compactor\\PhpScoper"
    ],
    "directories": [
        "bin",
        "src",
        "packages",
        "vendor"
    ],
    "output": "../rector.phar",
    "php-scoper": "../../build/scoper.inc.php"
}

I'm trying to debug this for over an hour, but no success.

Any clues where should I look?

Thank you

@TomasVotruba

This comment has been minimized.

Copy link
Author

commented Jan 23, 2019

Function in vendor is prefixed

namespace _HumbugBox310e0cf3f4f2\Safe;

function getcwd()
{
}
@TomasVotruba

This comment has been minimized.

Copy link
Author

commented Jan 23, 2019

The file is also in rector.phar/vendor/composer/autoload_static.php:

namespace Composer\Autoload;

class ComposerStaticInit0e959761b05d027ada79c01bca54cb8f
{
    public static $files = array (
        'f4817dcbd956cd221b1c31f6fbd5749c' => __DIR__ . '/..' . '/thecodingmachine/safe/generated/dir.php',
    )
}

(It's called dir, but there are all the dir related functions)

@theofidry

This comment has been minimized.

Copy link
Member

commented Jan 23, 2019

That... looks weird indeed.

First thing, dunno if you try it yet, is to check without the PHAR at all. You can easily do that by using the box compile --debug which will dump the content of the PHAR in .box_dump. That way if it works there, it means it has something to do with the PHAR itself, and if not, well at least it's easier to debug.

As a first attempt, maybe try to add: \file_put_contents('php://sdterr', 'HELLO'.\PHP_EOL); in the file in which _HumbugBox310e0cf3f4f2\Safe\getcwd() is defined to check if it is indeed autoloaded as expected.

If it has something to do with different process, it can be that maybe your functions are not autoloaded as expected.

@TomasVotruba

This comment has been minimized.

Copy link
Author

commented Jan 25, 2019

Thanks for you debug guide. I'll use these steps to get more info

@TomasVotruba TomasVotruba changed the title Is there something extra needed for autolaod of 3rd party functions? Is there something extra needed for autoload of 3rd party functions? Jan 25, 2019

@TomasVotruba

This comment has been minimized.

Copy link
Author

commented Jan 25, 2019

I try to isolate the process first with minimal setup possible, that would show this problem.

I made this repository: https://github.com/TomasVotruba/dummy-phar

I has:

  • the box setup
  • the php-scoper setup
  • the broken dependency

https://travis-ci.org/TomasVotruba/dummy-phar/jobs/484491953#L568-

I got stuck on this error:

File "/home/travis/build/TomasVotruba/dummy-phar/tmp/build/index.php" was e  
  xpected to exist.        

image

I have no idea what that means. Could you help me to make that repository run?

@TomasVotruba

This comment has been minimized.

Copy link
Author

commented Jan 25, 2019

@theofidry I tried adding the \file_put_contents('php://sdterr', 'HELLO'.\PHP_EOL); It's in compiled file in phar, but nothing is echoed :(

When I run the php .box_dump/bin/rector, the error is the same.

I'll check the build process before compilation.

@theofidry

This comment has been minimized.

Copy link
Member

commented Jan 29, 2019

Ah I see. I think it's a mis-use of the base-path. The base-path was pointing to tmp/build but executed from the project root, so it was pointing at... /path/to/dummy-phar/tmp/build in which there is nothing.

I think you're better off using the --working-dir option of the compile command.

@TomasVotruba

This comment has been minimized.

Copy link
Author

commented Feb 1, 2019

Well, in /path/to/dummy-phar/tmp/build is the cloned Rector.

I tried your suggestion: rectorphp/compiler#3
If fails with missing index.php. I have no idea what it means and why I need some index.php

@TomasVotruba

This comment has been minimized.

Copy link
Author

commented Feb 1, 2019

I got inspired in PHPStan-compiler, where the base-path is used with no problem: https://github.com/phpstan/phpstan-compiler/blob/e713eaac576603ea65733ac94eff1c3288655df8/build/box.json#L4

@theofidry

This comment has been minimized.

Copy link
Member

commented Feb 1, 2019

Hm I guess I'm not expressing myself correctly.

The base-path property is used to calculate all the relative paths. For example if the output-path is dist/myphar.phar, the output with your current config will be (unless the base path is absolute) {path-to-working-dir}/{base-path}/{output-path} so here /path/to/dummy-phar/tmp/build/dist/myphar.phar.

It is the same for all the collected files. So for example the composer.json that Box will try to look for will be /path/to/dummy-phar/tmp/build/composer.json which is wrong since your composer.json is in /path/to/dummy-phar/composer.json IIRC.

If fails with missing index.php. I have no idea what it means and why I need some index.php

This error might be a bit cryptic but I'm not sure how to improve it. Basically when you are building a PHAR index.php is the traditional default PHP file looked for as the PHAR main script. So I kept this default. So what it means here is that Box did not find any main script configured (because with your config, you would expect it to be inferred from the composer.json but it failed to find it cf. the explanation above) so it falls back on that index.php which is the default value.

@TomasVotruba

This comment has been minimized.

Copy link
Author

commented Feb 1, 2019

What should I change to make it work?

@theofidry

This comment has been minimized.

Copy link
Member

commented Feb 1, 2019

I would suggestion the following config:

{
    "compactors": [
        "KevinGH\\Box\\Compactor\\PhpScoper"
    ],
    "output": "dummy.phar"
}
  • I don't really see the point of cherry-picking which files to build like you do in the build bash script since there is already a filtering happening in Box so it will pick only the necessary files
  • For the same reason I don't see the need of composer require --no-update dg/composer-cleaner:^2.0 -d build. There is a relatively basic filtering happening in Box already but I don't think trying to go any further is worth the effort. Especially after compressing the PHAR (which I suggest to)
  • composer update --no-dev --classmap-authoritative -d build is unnecessary as Box will do it (it's an optional feature most of the time but absolutely necessary when using PhpScoper)
  • For the alias setting: if you have a need for it sure, otherwise don't bother.
  • For the banner : I'm not sure why bother disabling it, it helps out to figure out how the PHAR has been built if you ever happen to have to inspect the PHAR and it doesn't impact the PHAR size (it's 4-5 lines of comments at most?)
  • check-requirements optional as well although I have trouble to understand why people don't like to rely on it

And with those changes I would just do box compile, no bash script or nothing. Sure you can try to have control at every level of the PHAR building to optimize it to the maximum, but I really don't think it's worth the effort. That said it's purely personal and up to you.

@TomasVotruba

This comment has been minimized.

Copy link
Author

commented Feb 1, 2019

Could humbly I ask you to send PR to https://github.com/TomasVotruba/dummy-phar?

I try to copy-paste your suggestions, but more and more errors pop-up and I have no idea how to solve them. It's very frustrating and I don't think copy-pasting is good way to approach this. Working solution would help me much more and I would bother you with describing every option there is.

@theofidry

This comment has been minimized.

Copy link
Member

commented Feb 1, 2019

I'll try to check it out tonight

@TomasVotruba

This comment has been minimized.

Copy link
Author

commented Feb 1, 2019

Thank you, making this PR work would be awesome - rectorphp/compiler#3

There are some differences, like bin/bootstra.php and bin/container.php that I see is not-copied. Probably another magic convention.

@theofidry

This comment has been minimized.

Copy link
Member

commented Feb 2, 2019

There are some differences, like bin/bootstra.php and bin/container.php that I see is not-copied. Probably another magic convention.

I removed them because I couldn't find them in your branch. I'm not sure we are trying to build the PHAR the same way

@TomasVotruba

This comment has been minimized.

Copy link
Author

commented Feb 2, 2019

I've removed the safe package from Symplify and Rector, so it's not a blocker anymore + tagged them both. Didn't find time to test yet.

Here is the process to compile:

https://github.com/rectorphp/compiler/blob/06e6661e9fb80270c2af1cf5f17afe19e339cf6e/src/Command/CompileCommand.php#L105-L131

@TomasVotruba

This comment has been minimized.

Copy link
Author

commented Feb 2, 2019

I removed them because I couldn't find them in your branch

Not sure where you look, but they're here: https://github.com/rectorphp/rector/tree/master/bin

  • rectorphp/compiler compiles rectorphp/rector

You probably tried to compile rectorphp/comiler itself.

@TomasVotruba

This comment has been minimized.

Copy link
Author

commented Feb 2, 2019

Try cloning the repo and just running the command in Travis:
https://github.com/rectorphp/compiler/blob/06e6661e9fb80270c2af1cf5f17afe19e339cf6e/.travis.yml#L16

bin/console compile

Then try running phar file:

tmp/rector.phar -vvv # location has changed in another PRs, so it can be in root, `tmp` or `tmp/rector` directory

You'll see what is going on a what fails

@TomasVotruba

This comment has been minimized.

Copy link
Author

commented Feb 2, 2019

I've removed the Safe package and there shoudl be no function autolaod in the Rector project. So the getcwd is not an issue anymore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.