You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Neil Conway <ne...@gmail.com> on 2017/05/09 18:29:59 UTC
Review Request 59105: Allowed leading zeros in input to stout's
Version parser.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59105/
-----------------------------------------------------------
Review request for mesos and Benjamin Mahler.
Bugs: MESOS-7464
https://issues.apache.org/jira/browse/MESOS-7464
Repository: mesos
Description
-------
The original behavior was to allow leading zeros. When Version was
enhanced with more complete support for SemVer [1], it was also changed
to reject leading zeros in numeric components (version numbers and
prerelease identifiers). Although this change was consistent with the
SemVer spec (which mandates that such version strings "MUST" be
considered invalid), it breaks compatibility with the versions used by
some common software packages (e.g., Docker).
This commit reverts the change in behavior, so that leading zeros are
once again allowed. In the future, we might consider adding a "strict"
mode to the Version parser, and/or supporting an "options" scheme that
would allow the user to customize the version parser's behavior.
[1] 287556284d76e03c11cff3fc076224fe966096e0
Diffs
-----
3rdparty/stout/include/stout/version.hpp 9120e42eedcfb4fc5ee41f08354441bc10914baf
3rdparty/stout/tests/version_tests.cpp bce185ec493054ec81d0764088d04f3e4147303d
Diff: https://reviews.apache.org/r/59105/diff/1/
Testing
-------
`make check`
Thanks,
Neil Conway
Re: Review Request 59105: Allowed leading zeros in input to stout's
Version parser.
Posted by Neil Conway <ne...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59105/
-----------------------------------------------------------
(Updated May 10, 2017, 4:08 a.m.)
Review request for mesos and Benjamin Mahler.
Changes
-------
Split comment changes to a separate review.
Bugs: MESOS-7464
https://issues.apache.org/jira/browse/MESOS-7464
Repository: mesos
Description
-------
The original behavior was to allow leading zeros. When Version was
enhanced with more complete support for SemVer [1], it was also changed
to reject leading zeros in numeric components (version numbers and
prerelease identifiers). Although this change was consistent with the
SemVer spec (which mandates that such version strings "MUST" be
considered invalid), it breaks compatibility with the versions used by
some common software packages (e.g., Docker).
This commit reverts the change in behavior, so that leading zeros are
once again allowed. In the future, we might consider adding a "strict"
mode to the Version parser, and/or supporting an "options" scheme that
would allow the user to customize the version parser's behavior.
[1] 287556284d76e03c11cff3fc076224fe966096e0
Diffs (updated)
-----
3rdparty/stout/include/stout/version.hpp 4be722208329f838a3a3ebaa3b44affb2a6905f4
3rdparty/stout/tests/version_tests.cpp bce185ec493054ec81d0764088d04f3e4147303d
Diff: https://reviews.apache.org/r/59105/diff/2/
Changes: https://reviews.apache.org/r/59105/diff/1-2/
Testing
-------
`make check`
Thanks,
Neil Conway
Re: Review Request 59105: Allowed leading zeros in input to stout's
Version parser.
Posted by Neil Conway <ne...@gmail.com>.
> On May 10, 2017, 12:34 a.m., Benjamin Mahler wrote:
> > 3rdparty/stout/include/stout/version.hpp
> > Lines 53-57 (patched)
> > <https://reviews.apache.org/r/59105/diff/1/?file=1711707#file1711707line58>
> >
> > Why move this comment down in this patch? Seems unrelated
Well, I was adjusting the header comment to clarify the current behavioral differences with SemVer, so it increasingly seemed like the comment was out of place. I can split it into a separate commit though.
- Neil
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59105/#review174401
-----------------------------------------------------------
On May 9, 2017, 6:29 p.m., Neil Conway wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59105/
> -----------------------------------------------------------
>
> (Updated May 9, 2017, 6:29 p.m.)
>
>
> Review request for mesos and Benjamin Mahler.
>
>
> Bugs: MESOS-7464
> https://issues.apache.org/jira/browse/MESOS-7464
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The original behavior was to allow leading zeros. When Version was
> enhanced with more complete support for SemVer [1], it was also changed
> to reject leading zeros in numeric components (version numbers and
> prerelease identifiers). Although this change was consistent with the
> SemVer spec (which mandates that such version strings "MUST" be
> considered invalid), it breaks compatibility with the versions used by
> some common software packages (e.g., Docker).
>
> This commit reverts the change in behavior, so that leading zeros are
> once again allowed. In the future, we might consider adding a "strict"
> mode to the Version parser, and/or supporting an "options" scheme that
> would allow the user to customize the version parser's behavior.
>
> [1] 287556284d76e03c11cff3fc076224fe966096e0
>
>
> Diffs
> -----
>
> 3rdparty/stout/include/stout/version.hpp 9120e42eedcfb4fc5ee41f08354441bc10914baf
> 3rdparty/stout/tests/version_tests.cpp bce185ec493054ec81d0764088d04f3e4147303d
>
>
> Diff: https://reviews.apache.org/r/59105/diff/1/
>
>
> Testing
> -------
>
> `make check`
>
>
> Thanks,
>
> Neil Conway
>
>
Re: Review Request 59105: Allowed leading zeros in input to stout's
Version parser.
Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59105/#review174401
-----------------------------------------------------------
Fix it, then Ship it!
Sorry, didn't see this until Adam pinged me, I'm a bit behind on email. :)
3rdparty/stout/include/stout/version.hpp
Lines 53-57 (patched)
<https://reviews.apache.org/r/59105/#comment247536>
Why move this comment down in this patch? Seems unrelated
- Benjamin Mahler
On May 9, 2017, 6:29 p.m., Neil Conway wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59105/
> -----------------------------------------------------------
>
> (Updated May 9, 2017, 6:29 p.m.)
>
>
> Review request for mesos and Benjamin Mahler.
>
>
> Bugs: MESOS-7464
> https://issues.apache.org/jira/browse/MESOS-7464
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The original behavior was to allow leading zeros. When Version was
> enhanced with more complete support for SemVer [1], it was also changed
> to reject leading zeros in numeric components (version numbers and
> prerelease identifiers). Although this change was consistent with the
> SemVer spec (which mandates that such version strings "MUST" be
> considered invalid), it breaks compatibility with the versions used by
> some common software packages (e.g., Docker).
>
> This commit reverts the change in behavior, so that leading zeros are
> once again allowed. In the future, we might consider adding a "strict"
> mode to the Version parser, and/or supporting an "options" scheme that
> would allow the user to customize the version parser's behavior.
>
> [1] 287556284d76e03c11cff3fc076224fe966096e0
>
>
> Diffs
> -----
>
> 3rdparty/stout/include/stout/version.hpp 9120e42eedcfb4fc5ee41f08354441bc10914baf
> 3rdparty/stout/tests/version_tests.cpp bce185ec493054ec81d0764088d04f3e4147303d
>
>
> Diff: https://reviews.apache.org/r/59105/diff/1/
>
>
> Testing
> -------
>
> `make check`
>
>
> Thanks,
>
> Neil Conway
>
>