box-project / box

📦🚀 Fast, zero config application bundler with PHARs.
https://box-project.github.io/box
MIT License
1.14k stars 100 forks source link

Compiling on windows adds `#!/usr/bin/env php` to the output #168

Closed c33s closed 11 months ago

c33s commented 6 years ago
php build/puppet-enc.phar foo.example.com --env=prod --file=nodes.yaml.dist
#!/usr/bin/env php
---
parameters:
    customer: example
    role_enc: undef
environment: development
>summary>box.yaml ```yaml files: - composer.json directories: - bin - config - src - lib finder: - name: '*.*' in: vendor exclude: - .gitignore - .md - phpunit - Tester - Tests - tests compactors: - Herrera\Box\Compactor\Json - Herrera\Box\Compactor\Php compression: GZ main: bin/puppet-enc output: build/puppet-enc.phar stub: true git-commit: git_commit git-version: git_version git-tag: git_tag chmod: '0755' ```
theofidry commented 6 years ago

🤔 tbh I'm not sure what the fix is for windows. And there is two issues here:

And since I'm neither familiar with windows and I have too little time, I don't think there is going to be a windows support unless someone is actively helping out with it :/

c33s commented 6 years ago

why no appveyor? i have not that much time but i can of course test stuff on windows. hope that you will support windows, as the symfony ecosystem is very windows friendly

what is the main difference between 2.x and 3.x which can have something to do with this problem?

i thought it was the stub part:

? Generating new stub
  - Using shebang line: #!/usr/bin/env php
  - Using banner:
    > Generated by Humbug Box.
    >
    > @link https://github.com/humbug/box

but the stub of both versions contains the shebang.

2.x

#!/usr/bin/env php
<?php
/**
 * Generated by Box.
 *
 * @link https://github.com/herrera-io/php-box/
 */
if (class_exists('Phar')) {
Phar::mapPhar('default.phar');
require 'phar://' . __FILE__ . '/bin/puppet-enc';
}
__HALT_COMPILER(); ?>

3.x

#!/usr/bin/env php
<?php

/*
 * Generated by Humbug Box.
 *
 * @link https://github.com/humbug/box
 */

Phar::mapPhar('box-auto-generated-alias-5ae3610e43c14.phar');

require 'phar://box-auto-generated-alias-5ae3610e43c14.phar/.box/check_requirements.php';

require 'phar://box-auto-generated-alias-5ae3610e43c14.phar/bin/puppet-enc';

__HALT_COMPILER(); ?>
theofidry commented 6 years ago

why no appveyor? i have not that much time but i can of course test stuff on windows. hope that you will support windows, as the symfony ecosystem is very windows friendly

I'm not against the idea per se, it's just that without a windows machine to debug/fix stuff and very little time, adding this constraint is far from ideal.

That said if you or anyone is willing to help out with the Windows support I'll happily welcome it and I can enable an AppVeyor for the repo.

what is the main difference between 2.x and 3.x which can have something to do with this problem? i thought it was the stub part:

I thought too but Composer is using the same stub... So I have no idea... @MacFJA did you encounter a similar issue?

c33s commented 6 years ago
theofidry commented 6 years ago

box v2.x works

I believe you, but honestly I don't recall any windows related test on Box so it might just be that Kevin had his hands on a windows machine and making sure it was working well.

running the symfony app without phar php bin/puppet-enc works

👍 At least we're sure it's happening by packaging the app in the PHAR. Can you just check with all the files are included to ensure that it's not caused by a missing file in the PHAR? You can inspect the PHAR afterwards either in an idea like PhpStorm (as long as the PHAR has the .phar extension) or with box info --list

c33s commented 6 years ago

there it is:

bin/puppet-enc

symfony project:

#!/usr/bin/env php
<?php

use App\Kernel;
use App\Application;
use Symfony\Component\Console\Input\ArgvInput;
use Symfony\Component\Debug\Debug;
use Symfony\Component\Dotenv\Dotenv;
...

box 2.x phar:

<?php

use App\Kernel;
use App\Application;
use Symfony\Component\Console\Input\ArgvInput;
use Symfony\Component\Debug\Debug;
use Symfony\Component\Dotenv\Dotenv;
...

box 3.x:

#!/usr/bin/env php
<?php

use App\Kernel;
use App\Application;
use Symfony\Component\Console\Input\ArgvInput;
use Symfony\Component\Debug\Debug;
use Symfony\Component\Dotenv\Dotenv;

it looks like linux does not need the shebang inside of a phar file (which sounds logical at least to me), if the shebang is still added linux ignore or strips it. windows outputs it. so stripping the #!/usr/bin/env php is maybe mandatory

c33s commented 6 years ago

from the chat: test with shebang: ~ failed because this setting only controls the shebang from the stub.php.

the bug is, that the shebang line is not stripped from the users bin file

MacFJA commented 6 years ago

I thought too but Composer is using the same stub... So I have no idea... @MacFJA did you encounter a similar issue?

Yes, I encounter something similar (See this issue and this PR)
As you can see the solution was to allow to remove the shebang at the file start.

But I don't search how others phar handle this case

MacFJA commented 6 years ago

@c33s What is the first line of the file bin/puppet-enc ?

I just look at Composer phar creation, they remove on the fly the shebang of there entry file.

So it's not the stub that print this message but probably the required file

c33s commented 6 years ago

@MacFJA #!/usr/bin/env php see my comment above https://github.com/humbug/box/issues/168#issuecomment-385047602 there you have the source and the compiled results for box2 and box3

MacFJA commented 6 years ago

see my comment above #168 (comment) there you have the source and the compiled results for box2 and box3

Oh sorry :sweat_smile:

In box2 the shebang is removed (doc here, code here)
But it should also be the case here (See here)

c33s commented 6 years ago

already chatted with @theofidry, i think he knows were the bug is. i think he wrote that the file with the stripped shebang gets overwritten with files imported to the phar.

c33s commented 6 years ago

the bug still exists with version 3.0.0-alpha.4

in the phar file both bin file contain the shebang

c33s commented 6 years ago
? Setting replacement values
  + @git_commit@: 5f620495beb07c9304e4bd88dafa11e45224b1c5
  + @git_tag@: 1.0.1
  + @git_version@: 1.0.1
? Registering compactors
  + KevinGH\Box\Compactor\Json
  + KevinGH\Box\Compactor\Php
? Adding main file: puppet-enc/bin/puppet-enc
? Adding requirements checker
? Adding binary files
    > No file found
? Adding files
    > 1133 file(s)
? Generating new stub
  - No shebang line
  - Using banner:
    > Generated by Humbug Box.
    >
    > @link https://github.com/humbug/box
? Compressing with the algorithm "GZ"
The system cannot find the file specified.
? Setting file permissions to 493
* Done.
c33s commented 4 years ago

@theofidry please reopen bug still exist on 3.8.1 and on 3.8.4

verified the php files in pharfile.phar/bin all have the sheban line.

but now the output is correct Using shebang line: #!/usr/bin/env php

? Adding requirements checker
? Adding binary files
    > No file found
? Auto-discover files? No
? Exclude dev files? Yes
? Adding files
    > 1485 file(s)
? Generating new stub
  - Using shebang line: #!/usr/bin/env php
  - Using banner:
    > Generated by Humbug Box 3.8.4@120b0a3.
    >
    > @link https://github.com/humbug/box
? Dumping the Composer autoloader
? Removing the Composer dump artefacts
? Compressing with the algorithm "GZ"

edit: would be quite important because box2 doesn't work with php7.4 :(

theofidry commented 4 years ago

@c33s would you mind to give a recap on what the actual issue is? Because IIRC, having the shebang line (unless you disable it) on the stub is expected (and the shebang is removed from the main script)

c33s commented 4 years ago

@theofidry the issue is the line. it gets outputed on windows. every phar you create with this line will output this line if run on windows. as the phar should me platform agnostic i ask myself what the benefit of putting the line there is? in many cases it may only be confusing to see this output but it also can be the source of errors. for example a cli tool which should output xml, yaml or json will always output the shebang line so the output is invalid. maybe think about not outputing this line at all.

theofidry commented 4 years ago

as the phar should me platform agnostic i ask myself what the benefit of putting the line there is?

If I understood correctly ensuring it can be executed fine once made executable as opposed to have to do php foo.phar.

But I'm curious as to why this is doing this, since Composer does it as well

c33s commented 4 years ago

hm no composer is not doing it.

just had a look in composer.phar/bin/composer

<?php

if (PHP_SAPI !== 'cli' && PHP_SAPI !== 'phpdbg') {
    echo 'Warning: Composer should be invoked via the CLI version of PHP, not the '.PHP_SAPI.' SAPI'.PHP_EOL;
}

setlocale(LC_ALL, 'C');
require __DIR__.'/../src/bootstrap.php';

use Composer\Console\Application;
use Composer\XdebugHandler\XdebugHandler;

error_reporting(-1);

// Restart without Xdebug
...

i'ts not about not having shebang in the stub it's about not having it in all the files under bin. box2 strips the shebang line from there, box3 keeps it, which i think is causing the problem

theofidry commented 4 years ago

just had a look in composer.phar/bin/composer

That's the main script, not the stub. In other words it's not the first file executed by the PHAR so indeed it should not have a shebang (unlike when you use the non-PHAR version).

i'ts not about not having shebang in the stub it's about not having it in all the files under bin. box2 strips the shebang line from there, box3 keeps it, which i think is causing the problem

Box3 should not either for sure. It is done here but maybe either this regex is incorrect in your case or does not work as expected on Windows

c33s commented 4 years ago

yes the problem is that the shebang line is not removed from the php scripts in bin/ from the "entry" script in bin which is executed when the phar file is executed.

as soon as i edit the entry script file and strip the shebang from there the phar gets compiled correctly.

c33s commented 4 years ago

both regex patterns (yours and that from composer work on windows)

<?php

$content = file_get_contents('./bin/puppet-enc');

echo "###########################################################";
echo preg_replace('{^#!/usr/bin/env php\s*}', '', $content);
echo "###########################################################";
echo preg_replace('/^#!.*\s*/', '', $content);
echo "###########################################################";

looks like the code is simply not executed on windows

theofidry commented 4 years ago

@c33s I would need your help to debug that, because there is really no reason for it to not be executed on Windows: https://github.com/humbug/box/blob/master/src/Configuration/Configuration.php#L278

c33s commented 4 years ago

just tested the master checkout of box (not the phar file) and the stripping works. retrieveMainScriptContents correctly removes the shebang but in the final phar file the shebang is back in.

hmm haven't we already talked about that... i have a deja vu.

for some reason the original file is packaged over the stripped file.

c33s commented 4 years ago

completly removing box.json also does not help. do you have a gitlab account? then i can give you access to the puppet-enc code if that helps. does box works async? then it would make sense that it can happen, that the strip of shebang happens before the adding of the unaltered main file to phar.

my box.json (which would be cool the keep like that, as it is box2 compatible)

{
    "files": [
        "composer.json"
    ],
    "directories": [
        "bin",
        "config",
        "src",
        "lib"
    ],
    "finder": [
        {
            "name": "*.*",
            "in": "vendor",
            "exclude": [
                ".gitignore",
                ".md",
                "phpunit",
                "Tester",
                "Tests",
                "tests"
            ]
        }
    ],
    "compactors": [
        "Herrera\\Box\\Compactor\\Json",
        "Herrera\\Box\\Compactor\\Php"
    ],
    "compression": "GZ",
    "main": "bin/puppet-enc",
    "output": "build/puppet-enc.phar",
    "stub": true,
    "git-commit": "git_commit",
    "git-version": "git_version",
    "git-tag": "git_tag",
    "chmod": "0755"
}

my composer.json

{
    "type": "project",
    "license": "proprietary",
    "require": {
        "php": "^7.1.3",
        "ext-iconv": "*",
        "symfony/console": "^4.0",
        "symfony/flex": "^1.0",
        "symfony/framework-bundle": "^4.0",
        "symfony/lts": "^4@dev",
        "symfony/yaml": "^4.0",
        "symfony/monolog-bundle": "^3.2"
    },
    "require-dev": {
        "symfony/dotenv": "^4.0"
    },
    "config": {
        "preferred-install": {
            "*": "dist"
        },
        "sort-packages": true
    },
    "autoload": {
        "psr-4": {
            "App\\": "src/",
            "C33s\\": "lib/C33s"
        }
    },
    "autoload-dev": {
        "psr-4": {
            "App\\Tests\\": "tests/"
        }
    },
    "replace": {
        "symfony/polyfill-iconv": "*",
        "symfony/polyfill-php71": "*",
        "symfony/polyfill-php70": "*",
        "symfony/polyfill-php56": "*"
    },
    "scripts": {
        "auto-scripts": {
            "cache:clear": "symfony-cmd",
            "assets:install --symlink --relative %PUBLIC_DIR%": "symfony-cmd"
        },
        "post-install-cmd": [
            "@auto-scripts"
        ],
        "post-update-cmd": [
            "@auto-scripts"
        ]
    },
    "conflict": {
        "symfony/symfony": "*"
    },
    "extra": {
        "symfony": {
            "id": null,
            "allow-contrib": false
        }
    },
    "bin": [
        "bin/puppet-enc"
    ]
}
theofidry commented 4 years ago

do you have a gitlab account?

@theofidry

does box works async?

To an extend: once it collected all the files and their contents, it processes them (to apply the compactors) asynchronously (if the PHPScoper one is present) and then dump them to the FS (synchronously)

What may be more likely is that for some reasons the main script file remains included in the dumped files although it should be excluded. Could you try with in Compile, registering the main script after adding the files (L231 & 235)?

c33s commented 4 years ago

yes, moving $main = $this->registerMainScript($config, $box, $logger); after $this->addFiles($config, $box, $logger, $io); solves the problem. the generated phar file now does not output the shebang line, the correct main script is in the bin folder inside the phar file (no shebang line there)

c33s commented 4 years ago

have to verify if it really works. after integrating webmozart/assert, a composer update of the project and also removing the vendor i get the strange error:

Attempted to load class "Assert" from namespace "Webmozart\Assert".
  Did you forget a "use" statement for another namespace?

sometimes the phar files cannot be openend with phpstorm and sometimes they open. if i can view them with phpstorm the webmozart class only contains use statements but no "class" section (class {}) which is completly stripped.

i have to investigate this after some sleep.

c33s commented 4 years ago

just had webmozart/assert in my dev-requirements. this led to a Assert class having only usees inside but no class :)

theofidry commented 4 years ago

That's still quite curious tbh...

c33s commented 4 years ago

@theofidry should i open a separate ticket?

theofidry commented 4 years ago

Yes please :)

theofidry commented 11 months ago

I'm not sure this bug is still relevant, but I would expect different issues for Windows which I would prefer to address in dedicated issues if someone is willing to work on it. Hence, I'll be closing this issue now