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

Add before-script and after-script hooks #17

Closed
kelunik opened this issue Mar 24, 2016 · 20 comments
Closed

Add before-script and after-script hooks #17

kelunik opened this issue Mar 24, 2016 · 20 comments
Assignees

Comments

@kelunik
Copy link
Contributor

kelunik commented Mar 24, 2016

Would be nice if it could run git describe --tags optionally and define a version constant like PHAR_BUILDER_GIT_VERSION. It would remove the need to always change a file before tagging a new release.

Might as well be done by using a script to release new versions.

@MacFJA
Copy link
Owner

MacFJA commented Mar 24, 2016

I'm not sure to understand what you try to do 😕

@kelunik
Copy link
Contributor Author

kelunik commented Mar 24, 2016

I want to provide a command line command like acme-client version that shows the current version. In case it's a nightly build (not one from a tag), version with git describe --tags would be something like v0.2.4-3-ga05a4f8. Since the packaged version doesn't have any .git info available, I need to package it somehow.

I could use a script that gets that information and builds the phar afterwards, but it would be nice if there was support directly in here, which would avoid the need for an extra script.

@kelunik
Copy link
Contributor Author

kelunik commented Mar 25, 2016

My current proposal would be something like

"before-script": [
    "git describe --tags > build/version.txt"
],
"after-script": [
    "rm build/version.txt",
    "chmod +x build/acme-client.phar"
]

in the "extra" section in the composer.json.

@kelunik kelunik changed the title Add a possibility to add a version while building Add before-script and after-script hooks Mar 25, 2016
@MacFJA
Copy link
Owner

MacFJA commented Mar 25, 2016

Something similar to Composer event script, why not.

Another idea is to create a new constant in the phar stub (bootstrap) code.
something like:

$this->phar->setStub(
    '#!/usr/bin/env php' . PHP_EOL .
    '<?php Phar::mapPhar(); ' . 
    (null === $constant?'':'define(\'PHAR_VERSION\', \''.addslashes($constant).'\');') . 
    'include "phar://' . $this->alias . '/' . $this->stubFile .
    '"; __HALT_COMPILER(); ?>'
);

And in the console call:

$ phar-builder package --version=`git describe --tags`

But your proposal allow to have event based addition action/control which can be lot more flexible

MacFJA added a commit that referenced this issue Mar 26, 2016
@MacFJA
Copy link
Owner

MacFJA commented Mar 26, 2016

@kelunik, I made a first version of the hooks/event implementation (see e6ebea5).

I have to finish the documentation about it.

@MacFJA MacFJA self-assigned this Mar 26, 2016
@kelunik
Copy link
Contributor Author

kelunik commented Mar 27, 2016

PHP Fatal error: Uncaught Error: Class 'League\Event\Emitter' not found in /home/kelunik/GitHub/kelunik/acme-client/vendor/macfja/phar-builder/app/Application.php:68

Somehow, league/event doesn't get installed even if it's in the composer.json.

@kelunik
Copy link
Contributor Author

kelunik commented Mar 27, 2016

And it doesn't respect the output-dir configuration option anymore.

@kelunik
Copy link
Contributor Author

kelunik commented Mar 27, 2016

Also PHP Warning: Declaration of MacFJA\PharBuilder\Event\ComposerAwareEvent::named($name, $composer) should be compatible with League\Event\Event::named($name) in /home/kelunik/GitHub/kelunik/acme-client/vendor/macfja/phar-builder/app/Event/ComposerAwareEvent.php on line 15

@MacFJA
Copy link
Owner

MacFJA commented Mar 27, 2016

Also PHP Warning: Declaration of MacFJA\PharBuilder\Event\ComposerAwareEvent::named($name, $composer) should be compatible with League\Event\Event::named($name) in /home/kelunik/GitHub/kelunik/acme-client/vendor/macfja/phar-builder/app/Event/ComposerAwareEvent.php on line 15

It's the second times! I really need to found a way to detect this! (Correct in 97bfa5f by the way)

@kelunik
Copy link
Contributor Author

kelunik commented Mar 27, 2016

I think the warning is only printed in PHP 7?

@MacFJA
Copy link
Owner

MacFJA commented Mar 27, 2016

Nope, it's my PHP(s) configuration: the E_STRICT flag is missing in error_reporting of my php.ini

@kelunik
Copy link
Contributor Author

kelunik commented Mar 27, 2016

Did you have a look at the output dir?

@MacFJA
Copy link
Owner

MacFJA commented Mar 27, 2016

Yep, normally it's corrected with the commit e5bb1ec

@kelunik
Copy link
Contributor Author

kelunik commented May 27, 2016

"league/event": "^2.1" is missing as requirement.

@kelunik
Copy link
Contributor Author

kelunik commented May 27, 2016

And another thing afterwards:

Uncaught Error: Class 'Neutron\SignalHandler\SignalHandler' not found
in /acme-client/vendor/macfja/phar-builder/app/Event/EventManager.php:76

https://packagist.org/packages/neutron/signal-handler

@kelunik
Copy link
Contributor Author

kelunik commented May 27, 2016

After that, it fails with

Warning: require(phar:///acme-client/build/acme-client.phar/vendor/composer/../symfony/polyfill-mbstring/bootstrap.php): failed to open stream: phar error: "vendor/symfony/polyfill-mbstring/bootstrap.php" is not a file in phar "/acme-client/build/acme-client.phar" in phar:///acme-client/build/acme-client.phar/vendor/composer/autoload_real.php on line 65

@MacFJA
Copy link
Owner

MacFJA commented May 31, 2016

Are you sure to use the right branch for your test?

Those two dependencies are declared in the branch events+issue-17 but not on the master

@kelunik
Copy link
Contributor Author

kelunik commented Jun 1, 2016

That might be the case because I used dev-master#hash before, because the branch didn't work. I opened an issue: composer/composer#5397. I guess it's related to the + in the branch name.

@kelunik
Copy link
Contributor Author

kelunik commented Jun 2, 2016

Bug in Composer is gone now. Issue with polyfill-mbstring persists. It's required, because it's listed in autoload.files. I think we had that issue before? Do we need stubs for that?

@kelunik
Copy link
Contributor Author

kelunik commented Jun 2, 2016

#21 fixes this.

@MacFJA MacFJA mentioned this issue Jul 21, 2016
@MacFJA MacFJA closed this as completed Aug 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants