Everything

php

Home » Project » Newsgroups » php.internals » re2c version(s) »

re2c version(s)

php.internals

Dmitry Stogov

8 years ago

Hi,

I think, many core developers saw unexpected changes in "zend_labguages_scanner.c" or "var_unserializer.c" after rebuilds.

This occurs, because we use different versions of re2c, and some of them produce really different code.

They also embed version number into the generate source. Currently different files in PHP source tree generated by different re2c versions:

ext/json/json_scanner.c 0.16

ext/date/lib/parse_date.c 0.15.3

ext/date/lib/parse_iso_intervals.c 0.15.3

ext/pdo/pdo_sql_parser.c 0.16

ext/phar/phar_path_check.c 1.0.3

sapi/phpdbg/phpdbg_lexer.c 0.16

ext/standard/url_scanner_ex.c 0.16

ext/standard/var_unserializer.c 1.0.1

Zend/zend_ini_scanner.c 0.15

Zend/zend_language_scanner.c 1.0.1

I propose, to change build scripts (in master and PHP-7.3) to require at least re2c version 1.0.0 (it seems 1.0.0-1.0.3 produce the same result) and suppress version output into the generated files.

I'm not sure about timelib files.

Thanks. Dmitry.

Nikita Popov

8 years ago

On Fri, Jul 13, 2018 at 11:13 AM, Dmitry Stogov <dmitry@zend.com> wrote:

> Hi,
>
>
> I think, many core developers saw unexpected changes in
> "zend_labguages_scanner.c" or "var_unserializer.c" after rebuilds.
>
> This occurs, because we use different versions of re2c, and some of them
> produce really different code.
>
> They also embed version number into the generate source. Currently
> different files in PHP source tree generated by different re2c versions:
>
>
> ext/json/json_scanner.c 0.16
>
> ext/date/lib/parse_date.c 0.15.3
>
> ext/date/lib/parse_iso_intervals.c 0.15.3
>
> ext/pdo/pdo_sql_parser.c 0.16
>
> ext/phar/phar_path_check.c 1.0.3
>
> sapi/phpdbg/phpdbg_lexer.c 0.16
>
> ext/standard/url_scanner_ex.c 0.16
>
> ext/standard/var_unserializer.c 1.0.1
>
> Zend/zend_ini_scanner.c 0.15
>
> Zend/zend_language_scanner.c 1.0.1
>
>
> I propose, to change build scripts (in master and PHP-7.3) to require at
> least re2c version 1.0.0 (it seems 1.0.0-1.0.3 produce the same result) and
> suppress version output into the generated files.
>
>
> I'm not sure about timelib files.
>
>
> Thanks. Dmitry.
>

I don't think normalizing the version really solves anything. These files
should be dropped from version control entirely instead. Generated files do
not belong in version control.

re2c is widely available on Linux distros nowadays (probably specifically
because PHP uses it) and while there might have been historical ground to
bundle these generated files, there no longer is one.

Nikita

Derick Rethans

8 years ago

On Fri, 13 Jul 2018, Nikita Popov wrote:

> On Fri, Jul 13, 2018 at 11:13 AM, Dmitry Stogov <dmitry@zend.com> wrote:
>
> > I think, many core developers saw unexpected changes in
> > "zend_labguages_scanner.c" or "var_unserializer.c" after rebuilds.
> >
> > This occurs, because we use different versions of re2c, and some of them
> > produce really different code.
> >
> > They also embed version number into the generate source. Currently
> > different files in PHP source tree generated by different re2c versions:
> >
> >
> > ext/json/json_scanner.c 0.16
> >
> > ext/date/lib/parse_date.c 0.15.3
> >
> > ext/date/lib/parse_iso_intervals.c 0.15.3
> >
> > ext/pdo/pdo_sql_parser.c 0.16
> >
> > ext/phar/phar_path_check.c 1.0.3
> >
> > sapi/phpdbg/phpdbg_lexer.c 0.16
> >
> > ext/standard/url_scanner_ex.c 0.16
> >
> > ext/standard/var_unserializer.c 1.0.1
> >
> > Zend/zend_ini_scanner.c 0.15
> >
> > Zend/zend_language_scanner.c 1.0.1
> >
> >
> > I propose, to change build scripts (in master and PHP-7.3) to require at
> > least re2c version 1.0.0 (it seems 1.0.0-1.0.3 produce the same result) and
> > suppress version output into the generated files.
> >
> >
> > I'm not sure about timelib files.
> >
>
> I don't think normalizing the version really solves anything. These
> files should be dropped from version control entirely instead.
> Generated files do not belong in version control.

the timelib files *must* be generated with 0.15.3:
https://github.com/mongodb/mongo/blob/master/src/third_party/scripts/timelib_get_sources.sh#L10-L11

> re2c is widely available on Linux distros nowadays (probably
> specifically because PHP uses it) and while there might have been
> historical ground to bundle these generated files, there no longer is
> one.

The timelib files will need to be continued to be bundled. There is no
rule in the Makefile either (unlike zend_language parser(s)), so this
should not cause issues with GIT.

cheers,
Derick

--
https://derickrethans.nl | https://xdebug.org | https://dram.io
Like Xdebug? Consider a donation: https://xdebug.org/donate.php,
or become my Patron: https://www.patreon.com/derickr
twitter: @derickr and @xdebug

Stas Malyshev

8 years ago

Hi!

> the timelib files *must* be generated with 0.15.3:
> https://github.com/mongodb/mongo/blob/master/src/third_party/scripts/timelib_get_sources.sh#L10-L11

That comment says 0.16 was problematic. Is it still true for 1.0.*? Was
it reported to re2c?

--
Stas Malyshev
smalyshev@gmail.com

Dmitry Stogov

8 years ago

Hi Nikita,

OK. Dropping files and requirement for re2c 1.0.0. Right?

Thanks. Dmitry.

________________________________
From: Nikita Popov <nikita.ppv@gmail.com>
Sent: Friday, July 13, 2018 12:26:22 PM
To: Dmitry Stogov
Cc: PHP internals list; Stanislav Malyshev; derick@derickrethans.nl; Christoph M. Becker
Subject: Re: [PHP-DEV] re2c version(s)

On Fri, Jul 13, 2018 at 11:13 AM, Dmitry Stogov <dmitry@zend.com<mailto:dmitry@zend.com>> wrote:
Hi,

I think, many core developers saw unexpected changes in "zend_labguages_scanner.c" or "var_unserializer.c" after rebuilds.

This occurs, because we use different versions of re2c, and some of them produce really different code.

They also embed version number into the generate source. Currently different files in PHP source tree generated by different re2c versions:

ext/json/json_scanner.c 0.16

ext/date/lib/parse_date.c 0.15.3

ext/date/lib/parse_iso_intervals.c 0.15.3

ext/pdo/pdo_sql_parser.c 0.16

ext/phar/phar_path_check.c 1.0.3

sapi/phpdbg/phpdbg_lexer.c 0.16

ext/standard/url_scanner_ex.c 0.16

ext/standard/var_unserializer.c 1.0.1

Zend/zend_ini_scanner.c 0.15

Zend/zend_language_scanner.c 1.0.1

I propose, to change build scripts (in master and PHP-7.3) to require at least re2c version 1.0.0 (it seems 1.0.0-1.0.3 produce the same result) and suppress version output into the generated files.

I'm not sure about timelib files.

Thanks. Dmitry.

I don't think normalizing the version really solves anything. These files should be dropped from version control entirely instead. Generated files do not belong in version control.

re2c is widely available on Linux distros nowadays (probably specifically because PHP uses it) and while there might have been historical ground to bundle these generated files, there no longer is one.

Nikita

Sara Golemon

8 years ago

On Fri, Jul 13, 2018 at 7:00 AM, Dmitry Stogov <dmitry@zend.com> wrote:

> OK. Dropping files and requirement for re2c 1.0.0. Right?
>

+1 to removing them from git (except timelib as Derick noted).

The minimum requirement is reasonable in terms of reproducibility, but
should that be specific to master/7.3?

I ask, because my release builder for 7.2 (and 7.1 if Joe is using
Davey's builder) is using re2c 0.13.5 .   I don't imagine the risk of
updating re2c mid-relase is particularly high, but it is non-zero.

-Sara

Anatoliy Belsky

8 years ago

Hi,

> -----Original Message-----
> From: php@golemon.com <php@golemon.com> On Behalf Of Sara Golemon
> Sent: Friday, July 13, 2018 8:22 PM
> To: Dmitry Stogov <dmitry@zend.com>
> Cc: Nikita Popov <nikita.ppv@gmail.com>; PHP internals list
> <internals@lists.php.net>; Stanislav Malyshev <smalyshev@gmail.com>;
> derick@derickrethans.nl; Christoph M. Becker <cmbecker69@gmx.de>
> Subject: Re: [PHP-DEV] re2c version(s)
>
> On Fri, Jul 13, 2018 at 7:00 AM, Dmitry Stogov <dmitry@zend.com> wrote:
> > OK. Dropping files and requirement for re2c 1.0.0. Right?
> >
> +1 to removing them from git (except timelib as Derick noted).
>
> The minimum requirement is reasonable in terms of reproducibility, but should
> that be specific to master/7.3?
>

The tool version is in most cases irrelevant for the generated files. I recall there was some issues with re2c 0.16 and ext/date in 7.1, but that's the only case I could recall. But limiting the tool version seems hard for another reason - LTS distros won't upgrade them. Fe. Jessie still has re2c 0.13.5, bison 3.0.2 by default. We'd effectively force anyone to either upgrade their distros, or to use some workarounds like manually installing the required tools into a prefix and prepending it to the path.

> I ask, because my release builder for 7.2 (and 7.1 if Joe is using
> Davey's builder) is using re2c 0.13.5 .   I don't imagine the risk of
> updating re2c mid-relase is particularly high, but it is non-zero.
>

Normally RMs would define the tool versions that are expected to be stable and that are always used for the official tarballs. In the case of 7.0, it'd be always re2c 0.13.5 and bison 3.0.2, where ever I'd produce the tarball (even I don't always use Jessie for packaging). We probably should take this into the release process docs, if it isn't there already.

Following up on Dmitry's idea - what if we'd not remove the generated files, but instead would define some minimal tool versions that have to be used by default for the core? That however also needs to stand in unison with how the official tarballs are produced. Also, it might be possible, that some particular code needs some particular tool version, which should be stated explicitly.

Regards

Anatol

Sara Golemon

8 years ago

On Fri, Jul 13, 2018 at 3:08 PM, Anatol Belski <ab@php.net> wrote:

>> I ask, because my release builder for 7.2 (and 7.1 if Joe is using
>> Davey's builder) is using re2c 0.13.5 .   I don't imagine the risk of
>> updating re2c mid-relase is particularly high, but it is non-zero.
>>
> Normally RMs would define the tool versions that are expected
> to be stable and that are always used for the official tarballs.
> In the case of 7.0, it'd be always re2c 0.13.5 and bison 3.0.2,
> where ever I'd produce the tarball (even I don't always use
> Jessie for packaging). We probably should take this into the
> release process docs, if it isn't there already.
>

Well, whichever version we've settled on, I've updated
sgolemon/php-release to allow using arbitrary versions of re2c.
https://github.com/sgolemon/php-release/commit/9197257bc6f2f607476f950733a0c13ab5e3e867

If RE2C_VERSION is set to 1.0.3, it'll swap in the pre-built 1.0.3
binary I've included in the package.
Otherwise, it'll clone the source repo, checkout the requested tag (or
commit hash) and build that).

I'll plan on doing 7.2.9+ with 1.0.3 (unless we decide differently by then).
Remi should stick to 0.13.5 (default) for 7.2.8-final as previously stated.
Doing a dry-run now to see how it looks.

-Sara

Anatoliy Belsky

8 years ago

> -----Original Message-----
> From: php@golemon.com <php@golemon.com> On Behalf Of Sara Golemon
> Sent: Friday, July 13, 2018 10:27 PM
> To: Anatol Belski <ab@php.net>
> Cc: Dmitry Stogov <dmitry@zend.com>; Nikita Popov <nikita.ppv@gmail.com>;
> PHP internals list <internals@lists.php.net>; Stanislav Malyshev
> <smalyshev@gmail.com>; derick@derickrethans.nl; Christoph M. Becker
> <cmbecker69@gmx.de>
> Subject: Re: [PHP-DEV] re2c version(s)
>
> On Fri, Jul 13, 2018 at 3:08 PM, Anatol Belski <ab@php.net> wrote:
> >> I ask, because my release builder for 7.2 (and 7.1 if Joe is using
> >> Davey's builder) is using re2c 0.13.5 .   I don't imagine the risk of
> >> updating re2c mid-relase is particularly high, but it is non-zero.
> >>
> > Normally RMs would define the tool versions that are expected to be
> > stable and that are always used for the official tarballs.
> > In the case of 7.0, it'd be always re2c 0.13.5 and bison 3.0.2, where
> > ever I'd produce the tarball (even I don't always use Jessie for
> > packaging). We probably should take this into the release process
> > docs, if it isn't there already.
> >
> Well, whichever version we've settled on, I've updated sgolemon/php-release to
> allow using arbitrary versions of re2c.
> https://github.com/sgolemon/php-
> release/commit/9197257bc6f2f607476f950733a0c13ab5e3e867
>
> If RE2C_VERSION is set to 1.0.3, it'll swap in the pre-built 1.0.3 binary I've
> included in the package.
> Otherwise, it'll clone the source repo, checkout the requested tag (or commit
> hash) and build that).
>

As long as it suffices for the platform targeted for packaging, that'd work. I'd wonder, if we could use this approach to have the exact tools version on Travis. Then it'd be matching with what is tested and released. If that'd be ok, still the Stas concern about other platforms were to be respected. Perhaps we could integrate such a mechanism into the core, but it should fallback to available re2c, bison and whatever, if the given platform can't build the required tool version. As it would be for dev, perhaps it could be ignored, as a release would deliver the generated files. Or, we should still keep the generated files in the repo.

Regards

Anatol

Alice Wonder

8 years ago

On 07/13/2018 01:27 PM, Sara Golemon wrote:

> On Fri, Jul 13, 2018 at 3:08 PM, Anatol Belski <ab@php.net> wrote:

*snip*

> Well, whichever version we've settled on, I've updated
> sgolemon/php-release to allow using arbitrary versions of re2c.
> https://github.com/sgolemon/php-release/commit/9197257bc6f2f607476f950733a0c13ab5e3e867
>
> If RE2C_VERSION is set to 1.0.3, it'll swap in the pre-built 1.0.3
> binary I've included in the package.
> Otherwise, it'll clone the source repo, checkout the requested tag (or
> commit hash) and build that).
>
> I'll plan on doing 7.2.9+ with 1.0.3 (unless we decide differently by then).
> Remi should stick to 0.13.5 (default) for 7.2.8-final as previously stated.
> Doing a dry-run now to see how it looks.
>
> -Sara
>

I build 7.1.x packages for CentOS 7 linked against LibreSSL (I don't do
7.2 because I do every other major release)

I use re2c 0.14.3 because, well, that's what EPEL for CentOS 7 ships.

Is there a real world benefit to upgrading that to the 1.0.3 version in
my build system? (I suspect relatively easy to do)

Is it likely 7.3 will require newer re2c? My LibreSSL PHP packaging is
likely switches to 7.3.x branch when 7.3.2 is released.

Christoph Becker

8 years ago

On 13.07.2018 at 11:26, Nikita Popov wrote:

> On Fri, Jul 13, 2018 at 11:13 AM, Dmitry Stogov <dmitry@zend.com> wrote:
>
>> I propose, to change build scripts (in master and PHP-7.3) to require at
>> least re2c version 1.0.0 (it seems 1.0.0-1.0.3 produce the same result) and
>> suppress version output into the generated files.
>
> I don't think normalizing the version really solves anything. These files
> should be dropped from version control entirely instead. Generated files do
> not belong in version control.

+1

--
Christoph M. Becker

Stas Malyshev

8 years ago

Hi!

> re2c is widely available on Linux distros nowadays (probably

On Linux distros on common platforms (Intel/AMD) - sure. But what if you
need an uncommon platform, or one that does not run Linux? It's those
platforms where you'd have to build PHP from source (after all, PHP is
also widely available as a package on Linux distros anyway) and adding
another hassle of figuring out how to build a third-party tool - I don't
think it's a good service to the community.

--
Stas Malyshev
smalyshev@gmail.com

Stas Malyshev

8 years ago

Hi!

> On Linux distros on common platforms (Intel/AMD) - sure. But what if you
> need an uncommon platform, or one that does not run Linux? It's those
> platforms where you'd have to build PHP from source (after all, PHP is
> also widely available as a package on Linux distros anyway) and adding
> another hassle of figuring out how to build a third-party tool - I don't
> think it's a good service to the community.

BTW, looking at re2c source, I don't see any mention of Windows. Does it
even build on Windows? What would be prerequisites for it? I thought
Windows is still a supported platform? It won't be nice if one couldn't
build PHP from source on Windows anymore...

--
Stas Malyshev
smalyshev@gmail.com

Anatoliy Belsky

8 years ago

> -----Original Message-----
> From: Stanislav Malyshev <smalyshev@gmail.com>
> Sent: Friday, July 13, 2018 8:46 PM
> To: Nikita Popov <nikita.ppv@gmail.com>; Dmitry Stogov <dmitry@zend.com>
> Cc: PHP internals list <internals@lists.php.net>; derick@derickrethans.nl;
> Christoph M. Becker <cmbecker69@gmx.de>
> Subject: Re: [PHP-DEV] re2c version(s)
>
> Hi!
>
> > On Linux distros on common platforms (Intel/AMD) - sure. But what if
> > you need an uncommon platform, or one that does not run Linux? It's
> > those platforms where you'd have to build PHP from source (after all,
> > PHP is also widely available as a package on Linux distros anyway) and
> > adding another hassle of figuring out how to build a third-party tool
> > - I don't think it's a good service to the community.
>
> BTW, looking at re2c source, I don't see any mention of Windows. Does it even
> build on Windows? What would be prerequisites for it? I thought Windows is still
> a supported platform? It won't be nice if one couldn't build PHP from source on
> Windows anymore...
>

The binary SDK has undergone a major revamp in the last couple of years. It's now using the MSYS2 port, which works almost fine and is an active project. Before that, some tools like bison 2.4.1 was used, because there was no way to upgrade it. See https://github.com/Microsoft/php-sdk-binary-tools/tree/master/msys2/usr . I was repeatedly asking systems@ to move this repo to git.php.net, but unfortunately that didn't happen. Anyway, the approach is platform specific and was inspired by how mozbuild does it. One always has the full control on which tool versions are used and bad versions can be omitted.

The binary SDK for Windows currently uses bison 3.0.4 (3.0.5 in staging) and re2c 1.0.3 on all the branches. The exact versions run on AppVeyor and are tested continuously. Also every release is tested individually and snapshots from windows.php.net are tested, too. The binary SDK has all the tools that are needed to build PHP. The sources for Windows zipballs are distributed separately, which has its obvious reasons.

Regards

Anatol

Sara Golemon

8 years ago

On Fri, Jul 13, 2018 at 2:40 PM, Stanislav Malyshev <smalyshev@gmail.com> wrote:

>> re2c is widely available on Linux distros nowadays (probably
>
> On Linux distros on common platforms (Intel/AMD) - sure. But what if you
> need an uncommon platform, or one that does not run Linux? It's those
> platforms where you'd have to build PHP from source (after all, PHP is
> also widely available as a package on Linux distros anyway) and adding
> another hassle of figuring out how to build a third-party tool - I don't
> think it's a good service to the community.
>

I would offer that this is what official releases and/or snaps.php.net is for.

**Checks snaps.php.net**
...wait, is this not a thing anymore?

-Sara

Christoph Becker

8 years ago

On 13.07.2018 at 21:07, Sara Golemon wrote:

> **Checks snaps.php.net**
> ....wait, is this not a thing anymore?

IIRC, that was already gone before I got my php.net account.  Nowadays
users are supposed to checkout from Git.

--
Christoph M. Becker

Stas Malyshev

8 years ago

Hi!

> I would offer that this is what official releases and/or snaps.php.net is for.

Do you mean releases would contain the generated files but the regular
source won't? I am not sure then why - if we say our "official" source
has generated files, why keep VCS out of sync with the source? Also,
what if you want to build a version that has not been released yet? I
think keeping VCS in a state where it can not be built in a major system
is not a good thing.

--
Stas Malyshev
smalyshev@gmail.com

Nikita Popov

8 years ago

On Fri, Jul 13, 2018 at 8:40 PM, Stanislav Malyshev <smalyshev@gmail.com>
wrote:

> Hi!
>
> > re2c is widely available on Linux distros nowadays (probably
>
> On Linux distros on common platforms (Intel/AMD) - sure. But what if you
> need an uncommon platform, or one that does not run Linux? It's those
> platforms where you'd have to build PHP from source (after all, PHP is
> also widely available as a package on Linux distros anyway) and adding
> another hassle of figuring out how to build a third-party tool - I don't
> think it's a good service to the community.
>

We always bundle generated files in distributed sources, so end-users need
neither re2c nor bison to build releases.

re2c is not a problem for Windows either, it is bundled as part of the PHP
SDK.

Nikita

Stas Malyshev

8 years ago

Hi!

> re2c is not a problem for Windows either, it is bundled as part of the
> PHP SDK.

OK then, Windows is not a problem, that makes it better.

--
Stas Malyshev
smalyshev@gmail.com

Jan Ehrhardt

8 years ago

Stanislav Malyshev in php.internals (Fri, 13 Jul 2018 11:40:12 -0700):

>> re2c is widely available on Linux distros nowadays (probably
>
>On Linux distros on common platforms (Intel/AMD) - sure. But what if you
>need an uncommon platform, or one that does not run Linux? It's those
>platforms where you'd have to build PHP from source (after all, PHP is
>also widely available as a package on Linux distros anyway) and adding
>another hassle of figuring out how to build a third-party tool - I don't
>think it's a good service to the community.

Directadmin just makes the tarballs from php.net available at every new
release. re2c is not even installed on my Directadmin powered CentOS 6
systems and bison is not used in the build process.

The Directadmin admins surely will not be happy if the generated files
are removed from the tarballs. And the Directadmin users will probably
not be happy either, because chances are high that new releases will
have a time delay. Currently a new PHP release is distributed through
Directadmin on the day of release.

--
Jan

Sara Golemon

8 years ago

> On Jul 14, 2018, at 00:02, Jan Ehrhardt <phpdev@ehrhardt.nl> wrote:
>
> Stanislav Malyshev in php.internals (Fri, 13 Jul 2018 11:40:12 -0700):
>>> re2c is widely available on Linux distros nowadays (probably
>>
>> On Linux distros on common platforms (Intel/AMD) - sure. But what if you
>> need an uncommon platform, or one that does not run Linux? It's those
>> platforms where you'd have to build PHP from source (after all, PHP is
>> also widely available as a package on Linux distros anyway) and adding
>> another hassle of figuring out how to build a third-party tool - I don't
>> think it's a good service to the community.
>
> Directadmin just makes the tarballs from php.net available at every new
> release. re2c is not even installed on my Directadmin powered CentOS 6
> systems and bison is not used in the build process.
>
> The Directadmin admins surely will not be happy if the generated files
> are removed from the tarballs. And the Directadmin users will probably
> not be happy either, because chances are high that new releases will
> have a time delay. Currently a new PHP release is distributed through
> Directadmin on the day of release.

We're not talking about removing from tarballs, only git.  RMs will continue generating bison and re2c targets so that these are not build requirements for normal users.

-Sara

Zeev Suraski

8 years ago

> -----Original Message-----
> From: Nikita Popov [mailto:nikita.ppv@gmail.com]
> Sent: Friday, July 13, 2018 12:26 PM
> To: Dmitry Stogov <dmitry@zend.com>
> Cc: PHP internals list <internals@lists.php.net>; Stanislav Malyshev
> <smalyshev@gmail.com>; derick@derickrethans.nl; Christoph M. Becker
> <cmbecker69@gmx.de>
> Subject: Re: [PHP-DEV] re2c version(s)
>
> On Fri, Jul 13, 2018 at 11:13 AM, Dmitry Stogov <dmitry@zend.com> wrote:
>
> > Hi,
> >
> >
> > I think, many core developers saw unexpected changes in
> > "zend_labguages_scanner.c" or "var_unserializer.c" after rebuilds.
> >
> > This occurs, because we use different versions of re2c, and some of
> > them produce really different code.
> >
> > They also embed version number into the generate source. Currently
> > different files in PHP source tree generated by different re2c versions:
> >
> >
> > ext/json/json_scanner.c 0.16
> >
> > ext/date/lib/parse_date.c 0.15.3
> >
> > ext/date/lib/parse_iso_intervals.c 0.15.3
> >
> > ext/pdo/pdo_sql_parser.c 0.16
> >
> > ext/phar/phar_path_check.c 1.0.3
> >
> > sapi/phpdbg/phpdbg_lexer.c 0.16
> >
> > ext/standard/url_scanner_ex.c 0.16
> >
> > ext/standard/var_unserializer.c 1.0.1
> >
> > Zend/zend_ini_scanner.c 0.15
> >
> > Zend/zend_language_scanner.c 1.0.1
> >
> >
> > I propose, to change build scripts (in master and PHP-7.3) to require
> > at least re2c version 1.0.0 (it seems 1.0.0-1.0.3 produce the same
> > result) and suppress version output into the generated files.
> >
> >
> > I'm not sure about timelib files.
> >
> >
> > Thanks. Dmitry.
> >
>
> I don't think normalizing the version really solves anything. These files should be
> dropped from version control entirely instead. Generated files do not belong in
> version control.
>
> re2c is widely available on Linux distros nowadays (probably specifically because
> PHP uses it) and while there might have been historical ground to bundle these
> generated files, there no longer is one.

I agree that these files shouldn't be in version control, but I think normalizing the version does buy us something - it ensures (or at least tries to) that whomever does generate these files does so with a version that we believe is suitable for the job.  This is of course especially important for the RM's machine - but it's also important for whomever else might be building from a direct checkout and not a source package (I think the files we currently see in source control are indicative of that).

So why not do both - remove these files from version control, but also update the re2c requirements in configure and makedist..?

And of course we still want to bundle these in our distros - just not track them in our source control.

Zeev

Remi Collet

8 years ago

Le 13/07/2018 à 23:48, Zeev Suraski a écrit :

> So why not do both - remove these files from version control, but also update the re2c requirements in configure and makedist..?
>
> And of course we still want to bundle these in our distros - just not track them in our source control.

I agree

Perhaps we can also add all the generated files (including configure) in
the tagged versions, so the tag will have same content than the official
archive.


Remi

Sara Golemon

8 years ago

On Tue, Jul 17, 2018 at 1:04 AM, Remi Collet <remi@fedoraproject.org> wrote:

> Le 13/07/2018 à 23:48, Zeev Suraski a écrit :
> Perhaps we can also add all the generated files (including configure) in
> the tagged versions, so the tag will have same content than the official
> archive.
>

Ick, no.  That's the worst outcome IMO.  I don't think they need to be
in git, but if we're going to have them there during tags, then they
should always be there.  One way or the other, not some
middle-of-the-road thing.

-Sara

Zeev Suraski

8 years ago

On Tue, Jul 17, 2018 at 2:01 PM Sara Golemon <pollita@php.net> wrote:

> On Tue, Jul 17, 2018 at 1:04 AM, Remi Collet <remi@fedoraproject.org>
> wrote:
> > Le 13/07/2018 à 23:48, Zeev Suraski a écrit :
> > Perhaps we can also add all the generated files (including configure) in
> > the tagged versions, so the tag will have same content than the official
> > archive.
> >
> Ick, no.  That's the worst outcome IMO.  I don't think they need to be
> in git, but if we're going to have them there during tags, then they
> should always be there.  One way or the other, not some
> middle-of-the-road thing.

I can explain why I think that what I propose is the best outcome:
- It ensures that the correct versions of re2c are always used.
- It doesn't track generated files in source control.
- It allows users to build PHP from source on platforms where re2c is
unavailable.

Correct me if I'm wrong, but isn't that exactly what we do with
zend_language_parser.c?  makedist is responsible for generating it so that
it's available in distributions, but it isn't tracked.  Why would
zend_language_scanner.c be any different?  My guess is that it's probably
because at the time we moved to re2c it wasn't nearly as ubiquitous as it
is today and even most developers didn't have access to it, but now that's
changed.

I'm also fine with what Remi proposed which is adding these files
specifically to the source control at the time of tagging, in the spirit of
tracking everything that we actually end up releasing in source control
(and perhaps do that for zend_language_parser.c if we decide that this is
the right thing to do).

Either way - the first step (normalizing re2c versions - updating our r2ec
requirements) seems to make sense.  We can decide about whether or not we
track the generated files in git as we do today, only during tagging or not
at all independently of that.

Zeev

Nikita Popov

8 years ago

On Tue, Jul 17, 2018 at 1:51 PM, Zeev Suraski <vsuraski@gmail.com> wrote:

> On Tue, Jul 17, 2018 at 2:01 PM Sara Golemon <pollita@php.net> wrote:
>
> > On Tue, Jul 17, 2018 at 1:04 AM, Remi Collet <remi@fedoraproject.org>
> > wrote:
> > > Le 13/07/2018 à 23:48, Zeev Suraski a écrit :
> > > Perhaps we can also add all the generated files (including configure)
> in
> > > the tagged versions, so the tag will have same content than the
> official
> > > archive.
> > >
> > Ick, no.  That's the worst outcome IMO.  I don't think they need to be
> > in git, but if we're going to have them there during tags, then they
> > should always be there.  One way or the other, not some
> > middle-of-the-road thing.
>
>
> I can explain why I think that what I propose is the best outcome:
> - It ensures that the correct versions of re2c are always used.
> - It doesn't track generated files in source control.
> - It allows users to build PHP from source on platforms where re2c is
> unavailable.
>
> Correct me if I'm wrong, but isn't that exactly what we do with
> zend_language_parser.c?  makedist is responsible for generating it so that
> it's available in distributions, but it isn't tracked.  Why would
> zend_language_scanner.c be any different?  My guess is that it's probably
> because at the time we moved to re2c it wasn't nearly as ubiquitous as it
> is today and even most developers didn't have access to it, but now that's
> changed.
>
> I'm also fine with what Remi proposed which is adding these files
> specifically to the source control at the time of tagging, in the spirit of
> tracking everything that we actually end up releasing in source control
> (and perhaps do that for zend_language_parser.c if we decide that this is
> the right thing to do).
>
> Either way - the first step (normalizing re2c versions - updating our r2ec
> requirements) seems to make sense.  We can decide about whether or not we
> track the generated files in git as we do today, only during tagging or not
> at all independently of that.
>
> Zeev
>

I feel like we are all really in violent agreement that these files should
be dropped from git, and at this point I'm not even sure what the
discussion is about anymore. Let's wait until after PHP-7.3 branching in
two weeks and drop them at that point.

Normalizing the version numbers seems unnecessary after they are dropped --
at least Dmitry's original motivation for that was related exclusively to
the spurious diffs caused by different versions, which will no longer be an
issue.

Nikita

Zeev Suraski

8 years ago

On Tue, Jul 17, 2018 at 6:05 PM Nikita Popov <nikita.ppv@gmail.com> wrote:

> I feel like we are all really in violent agreement that these files should
> be dropped from git, and at this point I'm not even sure what the
> discussion is about anymore. Let's wait until after PHP-7.3 branching in
> two weeks and drop them at that point.
>
> Normalizing the version numbers seems unnecessary after they are dropped
> -- at least Dmitry's original motivation for that was related exclusively
> to the spurious diffs caused by different versions, which will no longer be
> an issue.
>

While we all agree that the files should be dropped from git - there
appears to be disagreement regarding what else we need to do in addition.
In my opinion if that's the only action we'd take then I don't think we
should do it and the status quo is actually better - as it would mean that
it will no longer be possible to build our packages in platforms that don't
have re2c available or typically installed.  It needs to happen hand in
hand with providing these files in the source packages, and also ensuring
that whatever boxes one uses to create the packages - as well as developers
who check out the source code directly from git - have an acceptable
version of re2c.  It may be that we can accept a wide range of re2c
versions (although if there are substantial differences in code perhaps
it's better to err on the side of caution).

I'm not sure why we're not simply following exactly what we're doing with
the parser.  We have a list of acceptable bison versions.  We check both in
configure and makedist against that list, and refuse to generate the parser
otherwise.  We don't track the generated .c file in source control - but we
do include it in distros to account for environments that don't typically
have bison installed.  Why not do exactly the same with the re2c scanner?

Zeev

Nikita Popov

8 years ago

On Tue, Jul 17, 2018 at 6:41 PM, Zeev Suraski <vsuraski@gmail.com> wrote:

> On Tue, Jul 17, 2018 at 6:05 PM Nikita Popov <nikita.ppv@gmail.com> wrote:
>
>> I feel like we are all really in violent agreement that these files
>> should be dropped from git, and at this point I'm not even sure what the
>> discussion is about anymore. Let's wait until after PHP-7.3 branching in
>> two weeks and drop them at that point.
>>
>> Normalizing the version numbers seems unnecessary after they are dropped
>> -- at least Dmitry's original motivation for that was related exclusively
>> to the spurious diffs caused by different versions, which will no longer be
>> an issue.
>>
>
> While we all agree that the files should be dropped from git - there
> appears to be disagreement regarding what else we need to do in addition.
> In my opinion if that's the only action we'd take then I don't think we
> should do it and the status quo is actually better - as it would mean that
> it will no longer be possible to build our packages in platforms that don't
> have re2c available or typically installed.  It needs to happen hand in
> hand with providing these files in the source packages, and also ensuring
> that whatever boxes one uses to create the packages - as well as developers
> who check out the source code directly from git - have an acceptable
> version of re2c.  It may be that we can accept a wide range of re2c
> versions (although if there are substantial differences in code perhaps
> it's better to err on the side of caution).
>
> I'm not sure why we're not simply following exactly what we're doing with
> the parser.  We have a list of acceptable bison versions.  We check both in
> configure and makedist against that list, and refuse to generate the parser
> otherwise.  We don't track the generated .c file in source control - but we
> do include it in distros to account for environments that don't typically
> have bison installed.  Why not do exactly the same with the re2c scanner?
>

Ah yes, *of course* the generated files will be part of distribution
tarballs, just like we do with all generated files (not just the parser,
but also configure.) While I forgot to write this in my original mail, it
has been mentioned already 4 days ago. So again, it seems like we're really
in total agreement here, just a matter of turning it into reality ;)

Nikita

Zeev Suraski

8 years ago

On Tue, Jul 17, 2018 at 7:57 PM Nikita Popov <nikita.ppv@gmail.com> wrote:

>
> Ah yes, *of course* the generated files will be part of distribution
> tarballs, just like we do with all generated files (not just the parser,
> but also configure.) While I forgot to write this in my original mail, it
> has been mentioned already 4 days ago. So again, it seems like we're really
> in total agreement here, just a matter of turning it into reality ;)
>

You know, you made me go back to Sara's email where she disagreed with me,
only to find she actually was disagreeing with Remi's proposal to track the
generated files for releases in git.  That's definitely not a hill to die
on for me :)

So all in all I think you're right, we agree on the important things:
1. Remove generated files from git
2. Keep them in source packages

What we seem to disagree on is that we should have a narrower list of
acceptable re2c versions determined by configure/makedist.  This isn't a
hill to die on for me either, although I think that narrowing it down is
better in terms of our ability to deliver a source package with confidence,
and be sure that everyone who's testing/using it is testing the same thing
(similar to how we do it with bison).  Arguably it's more important to be
on the safe wide with makedist than it is with configure.

Thanks,

Zeev