Re: Cherry-pick proposal

2020-04-29 Thread Tim Hudson
Any change to the review gate check we have in place now that lowers it
will certainly not get my support.

If anything, that check before code gets approved should be raised, not
lowered.

Tim.

On Thu, 30 Apr 2020, 1:24 am Salz, Rich,  wrote:

> I suspect that the primary motivation for this proposal is that PR’s
> against master often become stale because nobody on the project looks at
> them. And then submitters are told to rebase, fix conflicts, etc. It gets
> disheartening. If that is the motivation, then perhaps the project should
> address that root cause.  Here are two ideas:
>
>
>
>1. Mark’s scanning tool complains to the OTC if it has been “X” weeks
>without OTC action.  I would pick X as 2.
>2. Change the submission rules to be one non-author OTC member review
>and allow OTC/OMC to put a hold for discussion during the 24-hour freeze
>period. That discussion must be concluded, perhaps by a vote, within “Y”
>weeks (four?).
>
>
>
>
>
>
>


Re: CI build timeouts

2020-04-29 Thread Salz, Rich
  *   Disabling memory leak checking doesn’t sound like a great idea..

On that one platform, no?




Re: Revisiting tradition: branches and tags

2020-04-29 Thread Short, Todd
As someone who is currently playing around with QUIC branches based on the tags 
and branch names, I *always* screw things up while typing. I wouldn't mind if 
the  key were banned from tags and branch names.
--
-Todd Short
// tsh...@akamai.com
// “One if by land, two if by sea, three if by the Internet."

> On Apr 13, 2020, at 1:09 PM, Richard Levitte  wrote:
> 
> On Mon, 13 Apr 2020 10:48:35 +0200,
> Matt Caswell wrote:
>> On 11/04/2020 10:53, Dr. Matthias St. Pierre wrote:
>>> I love the new naming scheme, in particular the fact that it's 
>>> all-lowercase and does not
>>> mix dashes and underscores anymore. I don't recall how often I cursed about 
>>> the current
>>> scheme which is so typer unfriendly.
>>> 
>>> I'd like to see *all* stable branch names and release tags converted to 
>>> new-style uniformly
>>> (keeping the old names for compatibility), so we have an overall consistent 
>>> scheme and people
>>> don't need to switch back and forth between naming conventions in the 
>>> future when doing
>>> historical investigations. The old names could be deprecated for later 
>>> removal.
>> 
>> Yes - this aspect was my main hesitation about the proposed new format,
>> i.e. the fact that we have a set of existing tags/branch names.
>> Constantly changing between the new format and the old format as we look
>> at older branches/tags could be painful. Just creating new tags for all
>> the existing ones would be one way forward.
> 
> New tags is perfectly possible to do.
> 
>> Typing OpenSSL_1_1_1-stable and getting all the right upper/lower case
>> characters in the right place and making sure to use _ vs - as
>> appropriate is a challenge for my fingers which I constantly fail.
> 
> I constantly fail the *current* names.
> 
>> Is it possible to set up alias names for the historical branches?
> 
> It's possible yes.  The hard part is 1.1.1.  There *is* something
> called 'git symbolic-ref', but they can only be added in repos we have
> physical access to, so while can add those on our git server, and they
> will work, we cannot add them in github.
> 
> Ref git help symbolic-ref
> 
> Cheers,
> Richard ( still thinks it's worth making the change with 3.0 )
> 
>> 
>> Matt
>> 
>> 
>>> 
>>> Matthias
>>> 
>>> 
>>> 
 -Original Message-
 From: openssl-project  On Behalf Of 
 Richard Levitte
 Sent: Friday, April 10, 2020 8:28 PM
 To: openssl-project@openssl.org
 Subject: Revisiting tradition: branches and tags
 
 Once upon a time, there was CVS (*).
 
 The story could stop there, but since CVS was what was available,
 OpenSSL was versioned with CVS.
 
 Among limitations that came with CVS was the allowed syntax in tag and
 branch names; letters, digits, dashes and underscores.  At the time,
 eveyone that wanted to encode a version number in a tag had to convert
 periods to some other character.  We chose underscores, ending up with
 tags like this:
 
OpenSSL_0_9_7-beta2
OpenSSL_0_9_7a
 
 Furthermore, the culture that we have with git, where branches are
 being pulled between repositories...  where you can actually have a
 multitude of repositories and pull data between them, was very hard if
 not practically impossible with CVS.  So, we occasionally had feature
 branches for longer term work.  To distinguish our so called stable
 branches, we had branch names with '-stable' added at the end:
 
OpenSSL_0_9_7-stable
 
 We *could* have had something shorter, like 'OpenSSL_0_9_7xx', but
 I guess that was too easy to mix up with our letter releases.
 
 With git, however, there's no technical reason why we must maintain
 what was originally a technical limitation.  I therefore propose that
 we start using tags like this starting with OpenSSL 3.0:
 
openssl-3.0.0-alpha1
openssl-3.0.0-beta2
openssl-3.0.0
openssl-3.0.1
openssl-3.1.0
 
 This is a little more than just avoiding to change the periods with
 underscores.  However, if you look at the tar files we've released for
 a long time, that's the naming format they use, and I can see benefits
 in having the tags for a release match the tar file of the same
 release:
 
openssl-0.9.7a.tar.gz
openssl-0.9.7a.tar.gz.asc
openssl-0.9.7a.tar.gz.md5
 
 Future tar files would be numbered with the new version scheme, of
 course:
 
openssl-3.0.0-alpha1.tar.gz
openssl-3.0.0-beta2.tar.gz
openssl-3.0.0.tar.gz
openssl-3.0.1.tar.gz
openssl-3.1.0.tar.gz
 
 So what about release branches?  We could actually follow a very
 similar new pattern, just replace the last digit with, say, an 'x', to
 signify that this branch will contain a series of patch releases:
 
openssl-3.0.x
 
 In this branch, one would expect to see the tags 'openssl-3.0.0',

Re: Cherry-pick proposal

2020-04-29 Thread Salz, Rich
I suspect that the primary motivation for this proposal is that PR’s against 
master often become stale because nobody on the project looks at them. And then 
submitters are told to rebase, fix conflicts, etc. It gets disheartening. If 
that is the motivation, then perhaps the project should address that root 
cause.  Here are two ideas:


  1.  Mark’s scanning tool complains to the OTC if it has been “X” weeks 
without OTC action.  I would pick X as 2.
  2.  Change the submission rules to be one non-author OTC member review and 
allow OTC/OMC to put a hold for discussion during the 24-hour freeze period. 
That discussion must be concluded, perhaps by a vote, within “Y” weeks (four?).





Re: OpenSSL version 3.0.0-alpha1 published

2020-04-29 Thread Sergio NNX
  *   Windows 10 x64

  *   GCC 8.3.0 x86_64

$ openssl version -a

OpenSSL 3.0.0-alpha1 "23 Apr 2020" (Library: OpenSSL 3.0.0-alpha1 "23 Apr 2020")
built on: Fri Apr 24 18:14:53 2020 UTC
platform: mingw64
options:  bn(64,64)
compiler: /mingw/bin/gcc.exe -m64 -DWINVER=0x0501 -D_WIN32_WINNT=0x0501 
-D_WIN32_IE=0x0501 -D__PTW32_STATIC_LIB -D__PTW32_CLEANUP_C -m64 -O2 -pipe 
-mms-bitfields -fno-builtin -march=core2 -mtune=core2 -DL_ENDIAN 
-DOPENSSL_BUILDING_OPENSSL -DOPENSSL_PIC -DUNICODE -D_UNICODE 
-DWIN32_LEAN_AND_MEAN -D_MT -DZLIB -DNDEBUG -I/mingw/x86_64-pc-mingw32/include 
-I/mingw/x86_64-pc-mingw32/include/directx -I/mingw/include
OPENSSLDIR: "C:/OpenSSL"
ENGINESDIR: "C:/MinGW/lib/engines-3"
MODULESDIR: "C:/MinGW/lib/ossl-modules"
Seeding source: os-specific
CPUINFO: OPENSSL_ia32cap=0x7ffaf3bfffeb:0x29c67af


Some issued found:

on.obj crypto/cversion.c
In file included from include/openssl/macros.h:11,
 from include/openssl/opensslconf.h:14,
 from include/openssl/macros.h:10,
 from include/openssl/crypto.h:15,
 from include/internal/cryptlib.h:23,
 from crypto/cversion.c:10:
crypto/cversion.c: In function 'OpenSSL_version':
include/openssl/opensslv.h:91:54: error: expected ';' before numeric constant
 # define OPENSSL_VERSION_TEXT "OpenSSL 3.0.0-alpha1 "23 Apr 2020""
  ^~
crypto/cversion.c:50:16: note: in expansion of macro 'OPENSSL_VERSION_TEXT'
 return OPENSSL_VERSION_TEXT;
^~~~
make[1]: *** [crypto/libcrypto-lib-cversion.obj] Error 1
make[1]: Leaving directory `/src/openssl-3.0.0-alpha1'
make: *** [build_sw] Error 2




From: openssl-users  on behalf of OpenSSL 

Sent: Friday, 24 April 2020 12:29 AM
To: openssl-project@openssl.org ; OpenSSL User 
Support ML ; OpenSSL Announce ML 

Subject: OpenSSL version 3.0.0-alpha1 published

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256


   OpenSSL version 3.0 alpha 1 released
   

   OpenSSL - The Open Source toolkit for SSL/TLS
   https://www.openssl.org/

   OpenSSL 3.0 is currently in alpha.

   OpenSSL 3.0 alpha 1 has now been made available.

   Note: This OpenSSL pre-release has been provided for testing ONLY.
   It should NOT be used for security critical purposes.

   Specific notes on upgrading to OpenSSL 3.0 from previous versions, as well
   as known issues are available on the OpenSSL Wiki, here:

https://wiki.openssl.org/index.php/OpenSSL_3.0

   The alpha release is available for download via HTTPS and FTP from the
   following master locations (you can find the various FTP mirrors under
   https://www.openssl.org/source/mirror.html):

 * https://www.openssl.org/source/
 * ftp://ftp.openssl.org/source/

   The distribution file name is:

o openssl-3.0.0-alpha1.tar.gz
  Size: 9530120
  SHA1 checksum:  4db145d3d9c9d7bfaa7b2a1fe1670f7a3781bb06
  SHA256 checksum:  
9d5be9122194ad1d649254de5e72afd329252f134791389d0cef627b18ed9a57

   The checksums were calculated using the following commands:

openssl sha1 openssl-3.0.0-alpha1.tar.gz
openssl sha256 openssl-3.0.0-alpha1.tar.gz

   Please download and check this $LABEL release as soon as possible.
   To report a bug, open an issue on GitHub:

https://github.com/openssl/openssl/issues

   Please check the release notes and mailing lists to avoid duplicate
   reports of known issues. (Of course, the source is also available
   on GitHub.)

   Yours,

   The OpenSSL Project Team.

-BEGIN PGP SIGNATURE-

iQEzBAEBCAAdFiEEhlersmDwVrHlGQg52cTSbQ5gRJEFAl6hpQcACgkQ2cTSbQ5g
RJHvtggAp7XIxm/00amD4TijQhJqMmGsj0RXqwAeSd0gWDQCf78GX4zMIW/tTgvk
I3Mb67DsOR5gdPZN5TigyqRaXSIAzfb8ZT4Gs9lo/j8RUi5AmzT2RYexbRv6bF6E
cQ0OabM3rk4qi4njTi/YD9YihO6/pv7tWZkkfPsN547bfm7p7fwCrEHw02En5IW8
hyFhkpKfA3c8MEa96yLwjhkYRTAzUmxus/mNID+Ja3/VTCmHjd1c57SHFPq9noll
Wqzhs3jEhluZKHpwmSSA0KQh1ph0kh6fnKLEn3Oge5dYV3P+JrFCRfDEMsI1Nb/F
hIr11rxXNxtBRKUSlOUyJATZn0sV6g==
=uRpM
-END PGP SIGNATURE-


Re: Cherry-pick proposal

2020-04-29 Thread Tomas Mraz
That we do not run CI explicitly before such cherry picks does not mean
that there is no CI run at all. The CI is triggered when the cherry-
picked commit is merged. If we were checking the status of the 1.1.1
branch CI runs periodically (with a bot sending e-mails about failures
to openssl-project or another list perhaps?). We could then quickly fix
it if cherry-pick introduced a regression.

I also see only a single case where a cherry pick introduced regression
on the 1.1.1 branch during the April. So I do not think we have a
serious issue requiring the mandatory process change immediately.
On the other hand I have no hard problems with the Matthias' proposal
either.

Also perhaps we should just make a recommendation on which kind of
commits (although cherry-picking cleanly) should have a separate PR.
For example commits touching crypto implementations and the EVP layer
should have separate 1.1.1 PRs even though they cherry-pick cleanly. 

And of course there is no requirement to not do the cherry-pick PR even
if the cherry-pick is clean. It is up to the committer discretion to
decide whether it is needed or not.

Tomas

On Wed, 2020-04-29 at 13:28 +, Dr. Matthias St. Pierre wrote:
> I completely agree with Nicolas reasoning. But we should try to keep
> things simple and not
> add too many regulations. What do you think of the following
> formulation?
>  
> > 
> For change requests which target both the master and the
> OpenSSL_1_1_1-stable branch,
> the following procedure should be followed:
>  
> - First, a pull request needs to be opened against the master branch
> for discussion.
>   Only after that pull request has received the necessary amount of
> approvals,
>   a separate pull request can be opened  against the OpenSSL_1_1_1-
> stable branch.
>  
> - A separate pull request against the OpenSSL_1_1_1-stable branch is
> required.
>   This holds - contrary to common practice - even if the change can
> be cherry-picked
>   without conflicts from the master branch. The only exception from
> this rule are
>   changes which are considered 'CLA: trivial', like e.g.
> typographical fixes.
> > 
>  
> Matthias
>  
>  
> From: openssl-project  On Behalf
> Of Nicola Tuveri
> Sent: Wednesday, April 29, 2020 3:05 PM
> To: openssl-project@openssl.org
> Subject: Re: Cherry-pick proposal
>  
> I can agree it is a good idea to always require backport as a
> separate PR, with the following conditions:
> - unless it's a 1.1.1 only issue, we MUST always wait to open the
> backport-to-111 PR until after the master PR has been approved and
> merged (to avoid splitting the discussions among different PRs, which
> make review and revisiting our history very hard)
> - trivial documentation changes, such as fixing typos, can be
> exempted
> 
> It must be clear that although things have changed a lot in the inner
> plumbings, we have so far managed to keep crypto implementations very
> much in sync between 1.1.1 and master, by applying the policy of
> "first merge to master, then possibly backport".
> 
> What I am afraid of in Bernd's proposal, and recent discussions, is
> that committers might be tempted to open PRs changing implementations
> against 1.1.1 first (to avoid frequent rebases due to unrelated
> changes) and let the `master` PR stagnate indefinitely because it
> feels like too much hassle to keep up with the development pace of
> master if your PR collaterally changes certain files.
> 
> An example of what can go wrong if we open a 1.1.1 PR concurrently
> with a PR for master can be seen here:
> - `master` PR: https://github.com/openssl/openssl/pull/10828
> - `1.1.1` PR: https://github.com/openssl/openssl/pull/11411
> 
> I highlight the following problems related to the above example:
> - as you can see the `1.1.1` has been merged, even though the
> `master` one has stalled while discussing which implementation we
> should pick. (this was my fault, I should have applied the `hold`
> label after stating that my approval for 1.1.1 was conditional to
> approving the `master` counterpart)
> - discussion that is integral part of the decision process was split
> among the 2 PRs, with over 40 comments each
> - there is no clear link between the `master` PR and the `1.1.1` PR
> for the same feature (this makes it very difficult to review what is
> the state of the "main" PR, and if we or someone else in some months
> or years needs to go check why we did things the way we did, will
> have a hard time connecting the dots)
> 
> I also think that the proposal as written is a bit misleading: I
> would very like to keep seeing in 1.1.1 a majority of commits cherry-
> picked from commits merged to master, with explicit tags in the 1.1.1
> commit messages (this helps keeping the git history self-contained
> without a too strong dependency on GitHub).
> I would rephrase the proposal as follows:
> 
> Merge to 1.1.1 should only happen after 

RE: Cherry-pick proposal

2020-04-29 Thread Dr. Matthias St. Pierre
I completely agree with Nicolas reasoning. But we should try to keep things 
simple and not
add too many regulations. What do you think of the following formulation?

>
For change requests which target both the master and the OpenSSL_1_1_1-stable 
branch,
the following procedure should be followed:

- First, a pull request needs to be opened against the master branch for 
discussion.
  Only after that pull request has received the necessary amount of approvals,
  a separate pull request can be opened  against the OpenSSL_1_1_1-stable 
branch.

- A separate pull request against the OpenSSL_1_1_1-stable branch is required.
  This holds - contrary to common practice - even if the change can be 
cherry-picked
  without conflicts from the master branch. The only exception from this rule 
are
  changes which are considered 'CLA: trivial', like e.g. typographical fixes.
>

Matthias


From: openssl-project  On Behalf Of Nicola 
Tuveri
Sent: Wednesday, April 29, 2020 3:05 PM
To: openssl-project@openssl.org
Subject: Re: Cherry-pick proposal

I can agree it is a good idea to always require backport as a separate PR, with 
the following conditions:
- unless it's a 1.1.1 only issue, we MUST always wait to open the 
backport-to-111 PR until after the master PR has been approved and merged (to 
avoid splitting the discussions among different PRs, which make review and 
revisiting our history very hard)
- trivial documentation changes, such as fixing typos, can be exempted

It must be clear that although things have changed a lot in the inner 
plumbings, we have so far managed to keep crypto implementations very much in 
sync between 1.1.1 and master, by applying the policy of "first merge to 
master, then possibly backport".

What I am afraid of in Bernd's proposal, and recent discussions, is that 
committers might be tempted to open PRs changing implementations against 1.1.1 
first (to avoid frequent rebases due to unrelated changes) and let the `master` 
PR stagnate indefinitely because it feels like too much hassle to keep up with 
the development pace of master if your PR collaterally changes certain files.

An example of what can go wrong if we open a 1.1.1 PR concurrently with a PR 
for master can be seen here:
- `master` PR: https://github.com/openssl/openssl/pull/10828
- `1.1.1` PR: https://github.com/openssl/openssl/pull/11411

I highlight the following problems related to the above example:
- as you can see the `1.1.1` has been merged, even though the `master` one has 
stalled while discussing which implementation we should pick. (this was my 
fault, I should have applied the `hold` label after stating that my approval 
for 1.1.1 was conditional to approving the `master` counterpart)
- discussion that is integral part of the decision process was split among the 
2 PRs, with over 40 comments each
- there is no clear link between the `master` PR and the `1.1.1` PR for the 
same feature (this makes it very difficult to review what is the state of the 
"main" PR, and if we or someone else in some months or years needs to go check 
why we did things the way we did, will have a hard time connecting the dots)

I also think that the proposal as written is a bit misleading: I would very 
like to keep seeing in 1.1.1 a majority of commits cherry-picked from commits 
merged to master, with explicit tags in the 1.1.1 commit messages (this helps 
keeping the git history self-contained without a too strong dependency on 
GitHub).
I would rephrase the proposal as follows:

Merge to 1.1.1 should only happen after approval of a dedicated PR 
targeting the OpenSSL_1_1_1-stable branch.

Potential amendments that I'd like the OTC to consider are:
a) before the end of the sentence add ", with the optional exclusion of trivial 
documentation-only changes"
b) after the end of the sentence add "In composing backport pull requests, 
explicit cherry-picking (`git cherry-pick -x`) of relevant commits merged to 
`master` or another stable branch is recommended and encouraged whenever 
possible."
c) adopt a more general statement:

Merge to any stable branch should only happen after approval of a dedicated 
PR targeting specifically that branch.




So, in summary, I would agree with the proposal, as I definitely think Bernd 
has a good point about running the 1.1.1 CI for things we think should be 
backported, but requires careful consideration of additional requirements to 
avoid duplicating review efforts, splitting discussions that should be kept 
together, or disrupting our processes making 1.1.1 and master diverge more than 
necessary.


Cheers,


Nicola

On Wed, 29 Apr 2020 at 14:08, Matt Caswell 
mailto:m...@openssl.org>> wrote:

The OTC have received this proposal and a request that we vote on it:

I would like to request that we do not allow cherry-picks between master
and 1.1.1-stable because these two versions are now very different, if a

Re: Cherry-pick proposal

2020-04-29 Thread Richard Levitte
I find the idea of *mandatory* separate PRs for master and 1.1.1
awful.  There's still a lot of code that hasn't changed at all.  CMS,
BIO, ...

We already have the policy that if we get a clean cherry-pick, we go
with it, otherwise we make a separate PR.  I've followed that guiding
rule for a long time.

As for always starting with master, I would say that it depends.  If
an issue has been identified in 1.1.1, I usually submit a PR against
1.1.1, because that's usually cleaner, i.e. the person making the
issue can easily pick the fixing PR and try it out without having to
wade through unrelated 3.0 things that may or may not work for them.
Or to put it in harsher words, submitting a fix against master for an
issue found in 1.1.1 is *user* *unfriendly*.
With such a PR, I then cherry-pick to master if that's clean, or
submit an up-port PR.

Cheers,
Richard

On Wed, 29 Apr 2020 15:04:57 +0200,
Nicola Tuveri wrote:
> I can agree it is a good idea to always require backport as a separate PR, 
> with the following
> conditions:
> - unless it's a 1.1.1 only issue, we MUST always wait to open the 
> backport-to-111 PR until after
> the master PR has been approved and merged (to avoid splitting the 
> discussions among different
> PRs, which make review and revisiting our history very hard)
> - trivial documentation changes, such as fixing typos, can be exempted
> 
> It must be clear that although things have changed a lot in the inner 
> plumbings, we have so far
> managed to keep crypto implementations very much in sync between 1.1.1 and 
> master, by applying the
> policy of "first merge to master, then possibly backport".
> 
> What I am afraid of in Bernd's proposal, and recent discussions, is that 
> committers might be
> tempted to open PRs changing implementations against 1.1.1 first (to avoid 
> frequent rebases due to
> unrelated changes) and let the `master` PR stagnate indefinitely because it 
> feels like too much
> hassle to keep up with the development pace of master if your PR collaterally 
> changes certain
> files.
> 
> An example of what can go wrong if we open a 1.1.1 PR concurrently with a PR 
> for master can be
> seen here:
> - `master` PR: https://github.com/openssl/openssl/pull/10828
> - `1.1.1` PR: https://github.com/openssl/openssl/pull/11411
> 
> I highlight the following problems related to the above example:
> - as you can see the `1.1.1` has been merged, even though the `master` one 
> has stalled while
> discussing which implementation we should pick. (this was my fault, I should 
> have applied the
> `hold` label after stating that my approval for 1.1.1 was conditional to 
> approving the `master`
> counterpart)
> - discussion that is integral part of the decision process was split among 
> the 2 PRs, with over 40
> comments each
> - there is no clear link between the `master` PR and the `1.1.1` PR for the 
> same feature (this
> makes it very difficult to review what is the state of the "main" PR, and if 
> we or someone else in
> some months or years needs to go check why we did things the way we did, will 
> have a hard time
> connecting the dots)
> 
> I also think that the proposal as written is a bit misleading: I would very 
> like to keep seeing in
> 1.1.1 a majority of commits cherry-picked from commits merged to master, with 
> explicit tags in the
> 1.1.1 commit messages (this helps keeping the git history self-contained 
> without a too strong
> dependency on GitHub).
> I would rephrase the proposal as follows:
> 
>     Merge to 1.1.1 should only happen after approval of a dedicated PR 
> targeting the
> OpenSSL_1_1_1-stable branch.
> 
> Potential amendments that I'd like the OTC to consider are:
> a) before the end of the sentence add ", with the optional exclusion of 
> trivial documentation-only
> changes"
> b) after the end of the sentence add "In composing backport pull requests, 
> explicit cherry-picking
> (`git cherry-pick -x`) of relevant commits merged to `master` or another 
> stable branch is
> recommended and encouraged whenever possible."
> c) adopt a more general statement:
> 
>     Merge to any stable branch should only happen after approval of a 
> dedicated PR targeting
> specifically that branch.
> 
> So, in summary, I would agree with the proposal, as I definitely think Bernd 
> has a good point
> about running the 1.1.1 CI for things we think should be backported, but 
> requires careful
> consideration of additional requirements to avoid duplicating review efforts, 
> splitting
> discussions that should be kept together, or disrupting our processes making 
> 1.1.1 and master
> diverge more than necessary.
> 
> Cheers,
> 
> Nicola
> 
> On Wed, 29 Apr 2020 at 14:08, Matt Caswell  wrote:
> 
> The OTC have received this proposal and a request that we vote on it:
>
> I would like to request that we do not allow cherry-picks between master
> and 1.1.1-stable because these two versions are now very different, if a
> cherry-pick 

Re: Cherry-pick proposal

2020-04-29 Thread Nicola Tuveri
I think we changed enough things in the test infrastructure that there is a
chance of creating subtle failures by merging cherry-picked commits from
master directly.

>From the burden perspective, from my point of view having a separate PR
that ran all the CI without failures is actually a benefit: I can do some
minimal testing on my machine before the final merge, instead of having to
push a branch to my personal github fork to run travis and appveyor to test
weird build options on platforms I don't have access to.

If we stick to opening the PR for backporting only after master is
completely approved, there shouldn't be too big a burden for the original
reviewers either: we can use `fixup!` commits if trivial changes are
required to clearly highlight what was changed compared to the original PR
for master, and other than that it's just a matter of checking that nothing
else changed from the originally approved changes. Approvals conditional to
the CI passing can also help to further reduce the burden of the grace
period for backport PRs.


Nicola

On Wed, 29 Apr 2020 at 14:24, Dr Paul Dale  wrote:

> My concern is are 1.1.1 and 3.0 really all that different?
> The core is quite different but the cryptographic algorithms aren’t.  CMS,
> x509, …?
>
> I’d rather not introduce a burden where it isn’t necessary.
>
> Pauli
> --
> Dr Paul Dale | Distinguished Architect | Cryptographic Foundations
> Phone +61 7 3031 7217
> Oracle Australia
>
>
>
>
> On 29 Apr 2020, at 10:08 pm, Matt Caswell  wrote:
>
>
> The OTC have received this proposal and a request that we vote on it:
>
> I would like to request that we do not allow cherry-picks between master
> and 1.1.1-stable because these two versions are now very different, if a
> cherry-pick succeeds, there is no guarantee that the result will work.
> Because we have no CI for the cherry-pick. If a cherry-pick is needed, a
> new PR for 1.1.1 should be done and approved independently.
>
> Before starting a vote I'd like to provide opportunity for comments, and
> also what the vote text should be.
>
> Thanks
>
> Matt
>
>
>


Re: Cherry-pick proposal

2020-04-29 Thread Nicola Tuveri
I can agree it is a good idea to always require backport as a separate PR,
with the following conditions:
- unless it's a 1.1.1 only issue, we MUST always wait to open the
backport-to-111 PR until after the master PR has been approved and merged
(to avoid splitting the discussions among different PRs, which make review
and revisiting our history very hard)
- trivial documentation changes, such as fixing typos, can be exempted

It must be clear that although things have changed a lot in the inner
plumbings, we have so far managed to keep crypto implementations very much
in sync between 1.1.1 and master, by applying the policy of "first merge to
master, then possibly backport".

What I am afraid of in Bernd's proposal, and recent discussions, is that
committers might be tempted to open PRs changing implementations against
1.1.1 first (to avoid frequent rebases due to unrelated changes) and let
the `master` PR stagnate indefinitely because it feels like too much hassle
to keep up with the development pace of master if your PR collaterally
changes certain files.

An example of what can go wrong if we open a 1.1.1 PR concurrently with a
PR for master can be seen here:
- `master` PR: https://github.com/openssl/openssl/pull/10828
- `1.1.1` PR: https://github.com/openssl/openssl/pull/11411

I highlight the following problems related to the above example:
- as you can see the `1.1.1` has been merged, even though the `master` one
has stalled while discussing which implementation we should pick. (this was
my fault, I should have applied the `hold` label after stating that my
approval for 1.1.1 was conditional to approving the `master` counterpart)
- discussion that is integral part of the decision process was split among
the 2 PRs, with over 40 comments each
- there is no clear link between the `master` PR and the `1.1.1` PR for the
same feature (this makes it very difficult to review what is the state of
the "main" PR, and if we or someone else in some months or years needs to
go check why we did things the way we did, will have a hard time connecting
the dots)

I also think that the proposal as written is a bit misleading: I would very
like to keep seeing in 1.1.1 a majority of commits cherry-picked from
commits merged to master, with explicit tags in the 1.1.1 commit messages
(this helps keeping the git history self-contained without a too strong
dependency on GitHub).
I would rephrase the proposal as follows:

Merge to 1.1.1 should only happen after approval of a dedicated PR
targeting the OpenSSL_1_1_1-stable branch.

Potential amendments that I'd like the OTC to consider are:
a) before the end of the sentence add ", with the optional exclusion of
trivial documentation-only changes"
b) after the end of the sentence add "In composing backport pull requests,
explicit cherry-picking (`git cherry-pick -x`) of relevant commits merged
to `master` or another stable branch is recommended and encouraged whenever
possible."
c) adopt a more general statement:

Merge to any stable branch should only happen after approval of a
dedicated PR targeting specifically that branch.




So, in summary, I would agree with the proposal, as I definitely think
Bernd has a good point about running the 1.1.1 CI for things we think
should be backported, but requires careful consideration of additional
requirements to avoid duplicating review efforts, splitting discussions
that should be kept together, or disrupting our processes making 1.1.1 and
master diverge more than necessary.


Cheers,


Nicola

On Wed, 29 Apr 2020 at 14:08, Matt Caswell  wrote:

>
> The OTC have received this proposal and a request that we vote on it:
>
> I would like to request that we do not allow cherry-picks between master
> and 1.1.1-stable because these two versions are now very different, if a
> cherry-pick succeeds, there is no guarantee that the result will work.
> Because we have no CI for the cherry-pick. If a cherry-pick is needed, a
> new PR for 1.1.1 should be done and approved independently.
>
> Before starting a vote I'd like to provide opportunity for comments, and
> also what the vote text should be.
>
> Thanks
>
> Matt
>


Re: Cherry-pick proposal

2020-04-29 Thread Dr Paul Dale
My concern is are 1.1.1 and 3.0 really all that different?
The core is quite different but the cryptographic algorithms aren’t.  CMS, 
x509, …?

I’d rather not introduce a burden where it isn’t necessary.

Pauli
-- 
Dr Paul Dale | Distinguished Architect | Cryptographic Foundations 
Phone +61 7 3031 7217
Oracle Australia




> On 29 Apr 2020, at 10:08 pm, Matt Caswell  wrote:
> 
> 
> The OTC have received this proposal and a request that we vote on it:
> 
> I would like to request that we do not allow cherry-picks between master
> and 1.1.1-stable because these two versions are now very different, if a
> cherry-pick succeeds, there is no guarantee that the result will work.
> Because we have no CI for the cherry-pick. If a cherry-pick is needed, a
> new PR for 1.1.1 should be done and approved independently.
> 
> Before starting a vote I'd like to provide opportunity for comments, and
> also what the vote text should be.
> 
> Thanks
> 
> Matt



Cherry-pick proposal

2020-04-29 Thread Matt Caswell


The OTC have received this proposal and a request that we vote on it:

I would like to request that we do not allow cherry-picks between master
and 1.1.1-stable because these two versions are now very different, if a
cherry-pick succeeds, there is no guarantee that the result will work.
Because we have no CI for the cherry-pick. If a cherry-pick is needed, a
new PR for 1.1.1 should be done and approved independently.

Before starting a vote I'd like to provide opportunity for comments, and
also what the vote text should be.

Thanks

Matt