Make `--phar` reproducible

[sebastianbergmann]

The only thing that was missing for the build of PHPUnit's PHAR to become reproducible was to set the timestamps of the files in the PHAR to a consistent date.

I have implemented that in sebastianbergmann/phpunit@4627215 using seld/phar-utils.

However, I do not like that the PHAR is now changed after it has been created using PHPAB.

It would be nice if PHPAB would do this while it creates the PHAR. What do you think, Arne?

[sebastianbergmann]

CC @drupol

[theseer]

Interesting idea. I'll have a look at it.

I was hoping to not having to touch phpab anymore ;-)

[drupol]

Once again, my fault ... Sorry 😖

[theseer]

No worries. It's a very reasonable request :)

[theseer]

I just tried to implement this - and failed.

The current implementation uses iterators to add the files into the phar. Should be fairly straight forward to hook into this.

The docs on phar::buildFromIterator state an SplFileInfo should be returned. So I implemented my own wrapping IteratorIterator that is (verifiably) constructing my custom SplFileInfo for each entry.

But none of my overwritten methods are ever called.

I do not currently see any means to control the meta data in the phar at build time. The cli tool mentioned does a binary patch of the raw data. It's not actually using the phar API.

I'm not sure what to do here atm..

[theseer]

Okay, reading the php-src (https://github.com/php/php-src/blob/master/ext/phar/phar_object.c) helped shedding some light.

If I understand the inline docs for ::buildFromIterator correctly, ext/phar is always, no matter what, using the info from the actual file being added. I have no idea why there is support for SplFileInfo at all, as nothing is actually used from it.

There are basically two ways - despite the many different APIs - to add things into the phar: By having ext/phar get pretty much everything from the actual source file, or by using addFromString - which does not provide any means to set timestamps.

So, looks like for the time being, there is nothing I can do here - at least not by using the ext/phar-API: The time stamp has to be either "correct" before the file gets added into the phar or we have to patch the created phar file.

I could, in theory, include Jordi's library and introduce this as a post-processing step. But that's not much different from what you do now already.

[sebastianbergmann]

Thank you for looking into this.

[sebastianbergmann]

Maybe @nielsdos, who has done a bit of work on ext/phar recently, can help? It would be great if starting with PHP 8.4 it would be possible to set a timestamp for all files that are added to a PHAR using ext/phar.

[ndossche]

@sebastianbergmann how should the API look like? Something like setTimestamp(string $filename, int|DateTimeInterface $dateTime) ? That seems doable.

[sebastianbergmann]

Thank you for your reply, Niels. To be honest, I have not really thought about the API for this yet. The first thing that came to mind was an optional DateTimeImmutable argument for Phar::buildFromIterator() to set the timestamp for all files that are added to that timestamp. Or a Phar::setTimestampForAllFiles(DateTimeImmuable) method.

What do you think, Arne?

CC @Seldaek as this could, eventually, hopefully, at some point in the future, make Seld\PharUtils\Timestamps superfluous.

[theseer]

I'm not yet convinced that having a specific API for timestamps on ext/phar would be the best way to approach this, as that's rather limiting. Technically, there can be a lot of properties on a file or directory additionally to "only" the timestamp (which one actually? mtime? atime? ...?).

The most obvious thing for me to control meta data was to implement my own SplFileInfo using my own Iterator. It's rather unexpected that none of the methods of my object instance are called. Maybe that could be changed?

If we want to enhance the ext/phar API, we could consider adding an (optional) SplFileInfo to addFromString to control meta data as there currently is no way at all to specify properties for the respective entry.

[drupol]

Hello,

Glad to see this conversation happening, as the outcome of this is a definitive improvement for reproducible PHARs.

Adding @theofidry which might be interested to participate.

[ndossche]

What metadata besides timestamps do you want to control that you can't control now? Uid and gid come to my mind.

The most obvious thing for me to control meta data was to implement my own SplFileInfo using my own Iterator. It's rather unexpected that none of the methods of my object instance are called. Maybe that could be changed?

I think we probably should make this work. If you still have the reproduction test case please share it. Also, this is imo orthogonal to improving the API for Phar to allow overriding metadata.

If we want to enhance the ext/phar API, we could consider adding an (optional) SplFileInfo to addFromString to control meta data as there currently is no way at all to specify properties for the respective entry.

Not a fan of this particular idea. SplFileInfo is tied to an actual file normally, so that seems like abusing the class for the purpose of holding metadata.

[theofidry]

Hi!

What metadata besides timestamps do you want to control that you can't control now? Uid and gid come to my mind.

I do not know if there is other metadata that needs to be manipulated.

From a reproducibility perspective the only thing I'm missing is rather clarification of what can cause a PHAR give different signatures.

We had the case with PHPStan on several occasions that the content the PHAR is built looked identical but for the signature. We tried to look deeper into it but besides the signature, the content of the PHAR looked identical. The only difference I could note was when looking at the hexes, the order of the files looked different.

So to me it would be great to know exactly what defines the signature and, provided built the PHAR from the exact same source and the timestamp and algorithm are set, what could still make it that the built PHARs are not identical.

I assume the system filesystem could be a factor, or LC_COLLATE, or the compression algorithm that may not be deterministic? Would be great to have more information there.

A bit unrelated so I don't want to expend to much on it here, but I'm trying to compile some improvements I would like to see in the PHAR extension in box-project/box#1154 (it is entirely unpolished and probably missing some stuff still).

Back to the topic though, I would be more than content with Phar::setTimestamps(DateTimeImmutable): void, but it needs to respect/account for the signing algorithm. The problem is already solved in user-land with https://github.com/Seldaek/phar-utils (which has already been mentioned). The entire code in Box for it currently looks like this:

// src/Box.php

public function sign(
    SigningAlgorithm $signingAlgorithm,
    ?DateTimeImmutable $timestamp = null,
): void {
    if (null === $timestamp) {
        $this->phar->setSignatureAlgorithm($signingAlgorithm->value);

        return;
    }

    // Ensure all the changes done to the PHAR are "committed" before we manipulate the file
    $phar = $this->phar;
    $phar->__destruct();
    unset($this->phar);

    $util = new \Seld\PharUtils\Timestamps($this->pharFilePath);
    $util->updateTimestamps($timestamp);
    $util->save(
        $this->pharFilePath,
        $signingAlgorithm->value,
    );

    $this->phar = new Phar($this->pharFilePath);
}