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

[Config] Fixing GlobResource when inside phar archive #26845

Merged
merged 1 commit into from Jun 20, 2018

Conversation

Projects
None yet
6 participants
@vworldat
Contributor

vworldat commented Apr 6, 2018

Q A
Branch? master
Bug fix? yes
New feature? no
BC breaks? yes if old broken behavior counts as stable
Deprecations? no
Tests pass? no tests yet
Fixed tickets
License MIT
Doc PR N/A

When packaging an Sf4 application as a PHAR archive using globs at various locations (Kernel, services.yaml) most glob files are not found because the glob() PHP method does not support PHAR streams.

Using the regex fallback instead when operating inside PHAR archives fixes the behavior for me.

Examples:

src/Kernel.php::configureContainer():

$loader->load($confDir.'/{services}'.self::CONFIG_EXTS, 'glob');

Expected behavior: config/services.yaml inside PHAR archive is found and parsed
Actual behavior: the file will not be loaded

config/services.yaml (hard-coded in Kernel without using glob pattern)

    App\:
        resource: '../src/*'
        exclude: '../src/{Entity,Migrations,Tests,Kernel.php}'

Expected behavior: service classes in src/ will be found and auto-wired
Actual behavior: services are not auto-wired because the class files are not found

@c33s

This comment has been minimized.

c33s commented Apr 6, 2018

is it really a bc break? looks like a bug fix for me

@ogizanagi

This comment has been minimized.

Member

ogizanagi commented Apr 6, 2018

Actually, this should not be an issue if you warmup the prod cache (and thus build the container) before building your phar (and bundling var/cache/prod in it). The container would be built once and won't need to discover files again, so this is never hit.

As I answered @c33s on Slack, I use the following https://github.com/humbug/box config with no issue to bundle a Symfony app in a phar:

{
  "files": ["config/bundles.php"],
  "finder": [
    { "in": "src" },
    { "in": "var/cache/prod" },
    {
      "name": ["*.php"],
      "exclude": ["Tests", "Tester", "Test", "test", "tests"],
      "in": "vendor"
    }
  ],
  "main": "bin/phar-app",
  "output": "legacy-importer.phar",
  "compression": "GZ",
  "stub": true
}

where bin/phar-app is just a dedicated entrypoint setting the APP_ENV env var and configuring a single command application.

@vworldat

This comment has been minimized.

Contributor

vworldat commented Apr 6, 2018

@ogizanagi you are of course right, if pre-warming the cache is an option this can be used to work around the issue. Nevertheless I think the current behavior is not intended and leads to confusion as well as bad DX. And once someone might want to run a packaged app in debug mode for whatever reason, the pre-warmed cache may not be sufficient anymore.

@nicolas-grekas nicolas-grekas removed the BC Break label Apr 7, 2018

@nicolas-grekas nicolas-grekas added this to the 3.4 milestone Apr 7, 2018

@nicolas-grekas

This comment has been minimized.

Member

nicolas-grekas commented Apr 7, 2018

Could you add a test case please?

@c33s

This comment has been minimized.

c33s commented Apr 10, 2018

example.zip
@vworldat phar fixture file for the tests containing

enc/puppet-enc/build/example.phar
.phar
.phar/signature.bin
.phar/stub.php
bin
bin/console
config
config/packages
config/packages/dev
config/packages/dev/monolog.yaml
config/packages/dev/routing.yaml
config/packages/prod
config/packages/prod/monolog.yaml
config/packages/test
config/packages/test/framework.yaml
config/packages/test/monolog.yaml
config/packages/framework.yaml
config/packages/routing.yaml
config/bundles.php
config/routes.yaml
config/services.yaml
composer.json

@c33s c33s referenced this pull request Apr 27, 2018

Closed

Reduced config leads to error #167

@theofidry

This comment has been minimized.

Contributor

theofidry commented Apr 28, 2018

Could it also be that the application is executed in the wrong environment? I.e. when booting inside the PHAR, it detects that the dumped container is outdated and tries to dump it again (which will fail)

@nicolas-grekas

This comment has been minimized.

Member

nicolas-grekas commented Apr 30, 2018

@vworldat would you mind adding a test base, borrowing the fixtures phar from @c33s please?

@nicolas-grekas nicolas-grekas changed the base branch from master to 3.4 Jun 19, 2018

Fixing GlobResource when inside phar archive
When packaging an Sf4 application as a PHAR archive using globs at various locations (`Kernel`, `services.yaml`) most glob files are not found because the `glob()` PHP method [does not support PHAR streams](https://stackoverflow.com/questions/8203188/unexpected-problems-with-php-phar).

Using the regex fallback instead when operating inside PHAR archives fixes the behavior for me.
@nicolas-grekas

This comment has been minimized.

Member

nicolas-grekas commented Jun 20, 2018

Thank you @vworldat.

@nicolas-grekas nicolas-grekas merged commit e336ebe into symfony:3.4 Jun 20, 2018

2 of 3 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
fabbot.io Your code looks good.
Details

nicolas-grekas added a commit that referenced this pull request Jun 20, 2018

bug #26845 [Config] Fixing GlobResource when inside phar archive (vwo…
…rldat)

This PR was merged into the 3.4 branch.

Discussion
----------

[Config] Fixing GlobResource when inside phar archive

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | yes if old broken behavior counts as stable
| Deprecations? | no
| Tests pass?   | no tests yet
| Fixed tickets |
| License       | MIT
| Doc PR        | N/A

When packaging an Sf4 application as a PHAR archive using globs at various locations (`Kernel`, `services.yaml`) most glob files are not found because the `glob()` PHP method [does not support PHAR streams](https://stackoverflow.com/questions/8203188/unexpected-problems-with-php-phar).

Using the regex fallback instead when operating inside PHAR archives fixes the behavior for me.

## Examples:

`src/Kernel.php::configureContainer()`:

```php
$loader->load($confDir.'/{services}'.self::CONFIG_EXTS, 'glob');
```

Expected behavior: `config/services.yaml` inside PHAR archive is found and parsed
Actual behavior: the file will not be loaded

`config/services.yaml` (hard-coded in Kernel without using glob pattern)

```yaml
    App\:
        resource: '../src/*'
        exclude: '../src/{Entity,Migrations,Tests,Kernel.php}'
```

Expected behavior: service classes in `src/` will be found and auto-wired
Actual behavior: services are not auto-wired because the class files are not found

Commits
-------

e336ebe Fixing GlobResource when inside phar archive

This was referenced Jun 25, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment