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

Git inferred data is lost when compiling #460

Open
morozov opened this issue May 26, 2020 · 16 comments
Open

Git inferred data is lost when compiling #460

morozov opened this issue May 26, 2020 · 16 comments
Labels
Milestone

Comments

@morozov
Copy link

morozov commented May 26, 2020

Bug report

Question Answer
Box version Box version 3.8.4@120b0a3 2019-12-13 17:22:43 UTC
PHP version 7.4.5
Composer version 1.10.6
Platform with version Ubuntu
Github Repo https://gist.github.com/morozov/746307479bc41c6db34eebeebdee8565

If an application that uses composer/package-versions-deprecated is packaged using Box, the resulting package will contain an invalid root package version. Please see the full reproduction script below.

box.json.dist
{
   "output": "example.phar"
}
Output
$ git clone git@gist.github.com:746307479bc41c6db34eebeebdee8565.git
Cloning into '746307479bc41c6db34eebeebdee8565'...
remote: Enumerating objects: 7, done.
remote: Counting objects: 100% (7/7), done.
remote: Compressing objects: 100% (5/5), done.
remote: Total 7 (delta 0), reused 7 (delta 0), pack-reused 0
Receiving objects: 100% (7/7), done.

$ cd 746307479bc41c6db34eebeebdee8565
$ composer install
Loading composer repositories with package information
Installing dependencies (including require-dev) from lock file
Package operations: 1 install, 0 updates, 0 removals
 - Installing composer/package-versions-deprecated (1.8.0): Loading from cache
Generating autoload files
composer/package-versions-deprecated: Generating version class...
composer/package-versions-deprecated: ...done generating version class
1 package you are using is looking for funding.
Use the `composer fund` command to find out more!

$ box compile

   ____
  / __ )____  _  __
 / __  / __ \| |/_/
/ /_/ / /_/ />  <
/_____/\____/_/|_|


Box version 3.8.4@120b0a3 2019-12-13 17:22:43 UTC

// Loading the configuration file
// "/home/morozov/746307479bc41c6db34eebeebdee8565/box.json.dist".

🔨  Building the PHAR "/home/morozov/746307479bc41c6db34eebeebdee8565/example.phar"

? No compactor to register
? Adding main file: /home/morozov/746307479bc41c6db34eebeebdee8565/example.php
? Adding requirements checker
? Adding binary files
   > No file found
? Auto-discover files? Yes
? Exclude dev files? Yes
? Adding files
   > 8 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
? No compression
? Setting file permissions to 0755
* Done.

💡  1 recommendation found:
   - The "output" setting can be omitted since is set to its default value
No warning found.

// PHAR: 42 files (101.79KB)
// You can inspect the generated PHAR with the "info" command.

// Memory usage: 8.39MB (peak: 8.58MB), time: <1sec

$ php example.phar
No version set (parsed as 1.0.0)@

$ php example.php
dev-master@36878bf86275404814177973f78705e07522dbd1
@theofidry theofidry added the bug label May 26, 2020
@morozov
Copy link
Author

morozov commented May 28, 2020

As a temporary workaround, one could use the following command in the project directory to patch the package with the proper versions file:

$ php -dphar.readonly=0 -- /path/to/package.phar << 'EOF'
<?php
$versions = 'vendor/composer/package-versions-deprecated/src/PackageVersions/Versions.php';
$phar = new Phar($_SERVER['argv'][1]);
$phar[$versions] = file_get_contents($versions);
EOF

@theofidry
Copy link
Member

theofidry commented May 28, 2020

Wouldn't a better solution to migrate towards the Ocramius one? That one at least works fine with Box (since box uses it)

@morozov
Copy link
Author

morozov commented May 28, 2020

I'm not using composer/package-versions-deprecated directly. I use jean85/pretty-package-versions which switched from the original library (ocramius/package-versions) to the fork in 1.3 (see Jean85/pretty-package-versions#13).

Unless I did something wrong, when I tried downgrading to 1.2 which uses the original library, the issue was still reproducible, so I'm not sure if switching the package will necessarily solve the problem.

@theofidry are you sure that this is because of the underlying versions library?

@theofidry
Copy link
Member

Nope. It's however a suspicion since both are very close but one works and the other not. Since you gave a reproducer though, I'll be able to check it more closely

@morozov
Copy link
Author

morozov commented May 28, 2020

Wouldn't a better solution to migrate towards the Ocramius one?

As far as I can tell, Composer 2 is taking over the installed versions management.

@villfa
Copy link
Contributor

villfa commented Oct 1, 2020

Hi @morozov,
I've investigated your problem and I can explain why the version of the root package is lost.

The Composer's dump-autoload is invoked when the phar file is built. Because of the composer/package-versions-deprecated plugin the vendor/composer/package-versions-deprecated/src/PackageVersions/Versions.php file is re-generated.
The trick is that the generation occurs in a temporary folder containing your project except for certain files and folders. The .git/ folder for instance is excluded to keep the phar file light, but without it Composer cannot retrieve the version of the root package.

By tuning the configuration of box you can disable the call to dump-autoload in order to keep the original Versions.php:

box.json.dist:

{
    "dump-autoload": false,
    "finder": [
        {
            "name": "*.php",
            "in": "vendor"
        }
    ],
    "output": "example.phar"
}

And here the result I got:

$ ./example.phar
dev-master@36878bf86275404814177973f78705e07522dbd1

@theofidry
Copy link
Member

I was not aware it was relying on the .git/... It's a bit annoying actually since it's not files you want to include in the PHAR

@morozov
Copy link
Author

morozov commented Oct 1, 2020

@villfa, thank you for the research and the suggestion. The version detection indeed works this way but the resulting package now contains thevendor directory files that belong to the development dependencies. By default, Box seems to be smart enough to not include them in the package even if the development dependencies are installed.

If I install only the needed dependencies via composer install --no-dev, then this solution no longer works (the version still displays as "No version set (parsed as 1.0.0)@").

@morozov
Copy link
Author

morozov commented Oct 1, 2020

It's a bit annoying actually since it's not files you want to include in the PHAR

The same is about the development dependencies but somehow Box figures out that they don't need to be included.

@theofidry
Copy link
Member

Maybe the alternative solution is for Box to include .git in the temporary directory but discard it before shipping everything into the PHAR. It's not super nice since it's yet another hook to add, but that would solve the problem right?

The alternative obviously is to have the version dumper working without git... But that sounds like a lot more work

@morozov
Copy link
Author

morozov commented Oct 2, 2020

The alternative obviously is to have the version dumper working without git... But that sounds like a lot more work

That sounds unrealistic. The version of the root package is detected from VCS.

@villfa
Copy link
Contributor

villfa commented Oct 7, 2020

There is another way to make it work:

Box could retrieve the version of the root package by using Composer:

$ composer show --self --format=text
name     : example/example
descrip. :
keywords :
versions : * dev-master
type     : library
homepage :
source   : []  36878bf86275404814177973f78705e07522dbd1
dist     : []  36878bf86275404814177973f78705e07522dbd1
path     :
names    : example/example

requires
composer/package-versions-deprecated ^1.8

(when the format option equals json some information are missing like the commit hash)

Then Box can add, if not present, the version into the temporary composer.json file (see doc) so the information is available to Composer during dump-autoload.

@theofidry what do you think? If this proposal is good enough for you I could make a PR.

@theofidry
Copy link
Member

@villfa that could work, but is it really equivalent?

# Box not tagged via composer/package-versions-deprecated
3.x-dev@e65178c
# Box not tagged via the show command
versions : * 3.x-dev
source   : []  e65178c5edaf5012b8b6ae9137274f76a4b0a572
dist     : []  e65178c5edaf5012b8b6ae9137274f76a4b0a572

So from there we can reconstruct 3.x-dev@e65178c, but from the doc composer.json#version should match a specific format and 3.x-dev@e65178c is one of them.

After a quick look, maybe we are making this more complicated than it needs. We can add the git files to Configuration::create() ($binaryFilesAggregate) and when calling Compiler::commit() we can remove those files as part of the $dumpAutoload hook. WDYT?

@villfa
Copy link
Contributor

villfa commented Oct 9, 2020

I've made some tests to check if my idea was doable and it is not :/

We can add the git files to Configuration::create() ($binaryFilesAggregate) and when calling Compiler::commit() we can remove those files as part of the $dumpAutoload hook.

I'm a bit annoyed the solution sounds too git-centric, but I think it should work.

@theofidry
Copy link
Member

I'm a bit annoyed the solution sounds too git-centric, but I think it should work.

I share your feeling there but I guess if Composer is being git-centric here there is not much we can do :/

sidz added a commit to infection/infection that referenced this issue Nov 17, 2020
sanmai pushed a commit to infection/infection that referenced this issue Nov 18, 2020
…is (#1419)

* Update Box to 3.9.0 and remove temporary fix with composer v1 in travis

* update box to 3.9.1 which is compatible with composer 2

* replace psalm via phar because it loads composer/package-versions-deprecated which leads to box-project/box#460
@theofidry
Copy link
Member

Revisiting the issue: Since Composer relies on git and that some potential code do as well, at least when the autoloader is dumped as well, the planned fix is to include .git when building the PHAR but to not include it in the PHAR.

@theofidry theofidry changed the title Root package version gets lost during packaging Git inferred data is lost when compiling Jun 26, 2022
@theofidry theofidry added this to the Box5 milestone Nov 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants