ext/phar: inject timestamp for flushing

[Girgias]

The objective of this is to potentially allow userland to set the timestamp to allow building reproducible phars.

People this might interest in the long run:

• @theofidry
• @drupol
• @Ocramius

[drupol]

Thanks for this !

If I understand correctly, getting time from SOURCE_DATE_EPOCH is not an option here ?

[ndossche]

I also had this to deal with userland objects & reproducible phars, that came from a discussion in another's repo. #14143
However, no one of the original participants reacted to this after all, so ig the reproducible phar interest isn't as large as the crowd makes it out to be ;)

I'd have to think how this all fits together with reproducibility etc and if this is the "easiest" way etc...

[theofidry]

I vaguely remember #14143, I admit I'm a bit lost there I would need to dig back into that. But vague understanding is that it is about getting the correct value for the timestamp instead whatever value the PHAR was created at.

My naïve understand of the issue is that at the moment when you build a PHAR there is simply no API to set the timestamp.

For context, what most people use in practice in PHP land is use seld/phar-utils which reads the file and look up at the position to directly update the timestamp there, with the caveat that it needs to re-sign the PHAR.

So ideally would be cool to simply be able to do $phar->setTimestamp() somehow. This way you preserve the existing behaviour and do not break anything by default but users can set it to SOURCE_DATE_EPOCH or other for the sake of reproducible builds. The question would be how you deal with the signing though... Ideally, it would respect it, or you could say it needs to be set before signing but then signing doesn't override it (well actually I don't think it does, it's just because phar-utils changes the content after signing, so maybe there is nothing to do there...)

[hakre]

If buildFromIterator() would have a parameter to specify the timestamp, this would be so golden. This is a longtime issue for me when I archive inflight (build) trees. @Girgias Thanks a lot for all already, even if not yet that. /cc @Seldaek FYI.

[hakre]

Thanks for this !

Over 9000!

If I understand correctly, getting time from SOURCE_DATE_EPOCH is not an option here ?

IIUC that belongs to PHP script code getenv()/$_SERVER/etc. calls, correct.

see if you can avoid implementing it (ref)

^ this is what you normally want. and here it goes in (if it makes it). this is great! @drupol

/E: current example there (ref)

\date('Y', (int)\getenv('SOURCE_DATE_EPOCH') ?: \time())

[ndossche]

I vaguely remember #14143, I admit I'm a bit lost there I would need to dig back into that. But vague understanding is that it is about getting the correct value for the timestamp instead whatever value the PHAR was created at.

The PR allows you to override the implementations of getMTime() etc such that they are called instead of using the default one.

So ideally would be cool to simply be able to do $phar->setTimestamp() somehow. This way you preserve the existing behaviour and do not break anything by default but users can set it to SOURCE_DATE_EPOCH or other for the sake of reproducible builds.

Right. And setTimestamp() on its own is likely not enough. Ideally you have setTimestamp() on the Phar class itself, and also a setMTime() for the phar file info object.

If buildFromIterator() would have a parameter to specify the timestamp, this would be so golden.

That's a bit ad-hoc and won't cover phars built by other means.
I think the suggestion to add setTimestamp() & setMTime() is better as it will cover your use-case and others as well.

[Seldaek]

I think the suggestion to add setTimestamp() & setMTime() is better as it will cover your use-case and others as well.

That sounds good. In Composer fwiw we only add files using Phar::addFromString as we do some preprocessing ("compression" by removing comments from the code). Having a way to override the file timestamps either using setter after or just a unique filemtime for the whole phar sounds good to me. If I can delete that hackish phar-utils lib in some years I'll be glad :)

Given this is a rarely used API I don't think the ergonomics matter a whole lot here, but thanks for working on this in whichever form it ends up taking.

[hakre]

Now remembering file mode. If we touch mtime, we should probably also offer someting here as this can also effect reproducibility. Don't want to bikeshed here or something,, for fully reproducible phars over the PHP epoche, these need to be changed.

[sebastianbergmann]

Subscribed, since I use seld/phar-utils to build PHPUnit's PHAR. Thank you for working on this, @Girgias and @ndossche!

[Girgias]

I also had this to deal with userland objects & reproducible phars, that came from a discussion in another's repo. #14143 However, no one of the original participants reacted to this after all, so ig the reproducible phar interest isn't as large as the crowd makes it out to be ;)

I'd have to think how this all fits together with reproducibility etc and if this is the "easiest" way etc...

I guess, the issue is that there might be other metadata that should also be controllable from the user?

I'm happy to extend this by passing along some sort of "metadata" struct (possibly just timestamp + flags?) to be applied to all files. But I don't know what are the things that need control.

[hakre]

I also had this to deal with userland objects & reproducible phars, that came from a discussion in another's repo. #14143 However, no one of the original participants reacted to this after all, so ig the reproducible phar interest isn't as large as the crowd makes it out to be ;)
I'd have to think how this all fits together with reproducibility etc and if this is the "easiest" way etc...

I guess, the issue is that there might be other metadata that should also be controllable from the user?

I'm happy to extend this by passing along some sort of "metadata" struct (possibly just timestamp + flags?) to be applied to all files. But I don't know what are the things that need control.

Instead of accepting only a filename (as before), why not also allow the stringable SplFileInfo public interface for mtime and other file meta-data?

[Girgias]

Frankly, I don't want to couple more SPL jank with Phar.

I would rather create a new final PharDataMetaData class that contains a few properties that can override file metadata than piggy-back on SplFileInfo...

[hakre]

Frankly, I don't want to couple more SPL jank with Phar.

I would rather create a new final PharDataMetaData class that contains a few properties that can override file metadata than piggy-back on SplFileInfo...

Understandable, but doesn't exactly that lead to that anti-pattern? Normally we can't fight the confusion with that, despite this is what we all want, right?

/E: As I commented above, most important for me is getting the code churn. The chatter is just food for thought, the programmer should practice as they feel best and comfortable.

[Girgias]

Frankly, I don't want to couple more SPL jank with Phar.
I would rather create a new final PharDataMetaData class that contains a few properties that can override file metadata than piggy-back on SplFileInfo...

Understandable, but doesn't exactly that lead to that anti-pattern? Normally we can't fight the confusion with that, despite this is what we all want, right?

I don't understand what antipattern you are talking about.

[hakre]

I don't understand what antipattern you are talking about.

Most prominently Big Ball of Mud and Piecemeal Growth (ebd.) came to my mind. @Girgias