openssl / tools Goto Github PK

When executing do-release.pl the script does not move old 1.1.1 versions into the "old" directory like it does for 1.1.0 and 1.0.2

Now that openssl/openssl#6650 has been merged we have a test capable of testing GOST ciphersuites if a GOST engine in present. The test requires that the environment variable OPENSSL_GOST_ENGINE_SO is set to point at gost.so. We should set up run-checker so that this is configured.

See also openssl/openssl#21629

It was decided that the main repository needed the extra pair of eyes unconditionally.
However, it's not certain that the web and tools repos were under the same kind of consideration.

I am opening the issue here supposing that there is some script in here that is used to update the manmaster section of the website.

A full installation of current master would create manpages for the various commands, including e.g.
https://www.openssl.org/docs/manmaster/man1/openssl-pkeyutl.html
but not all manpages are being loaded to the web server.

Ping @mattcaswell /@levitte as this seems to be quite relevant for the imminent alpha1 release.

When using addrev just now, I got this big fat warning:

WARNING: git-filter-branch has a glut of gotchas generating mangled history
	 rewrites.  Hit Ctrl-C before proceeding to abort, then use an
	 alternative filtering tool such as 'git filter-repo'
	 (https://github.com/newren/git-filter-repo/) instead.  See the
	 filter-branch manual page for more details; to squelch this warning,
	 set FILTER_BRANCH_SQUELCH_WARNING=1.
Proceeding with filter-branch...

It took about a second or two before the last line appeared.

I didn't interrupt this because I'm confident in what addrev does and that I can recover from mistakes ('cause I've done them), but addrev should probably be reworked to use filter-repo anyway.

The "merged from" line is wrong if not in the openssl repo:

- Log -----------------------------------------------------------------
commit fe185a2b8f12669f7a9a88582cb63ad316cd2382
Author: Dr. Matthias St. Pierre <[email protected]>
Date:   Wed Aug 14 01:24:55 2019 +0200

...    
    Reviewed-by: Richard Levitte <[email protected]>
    (Merged from https://github.com/openssl/openssl/pull/40)

During the release today the do-release.pl script did not correctly move the old 1.1.0 and 1.0.2 files out of the source directory and into the "old" archive.

gitaddrev (regardless of any parameters provided) gives me this:

Use of uninitialized value in concatenation (.) or string at /Library/Perl/5.30/OpenSSL/Query/PersonREST.pm line 75.
Server error: Can't verify SSL peers without knowing which Certificate Authorities to trust at /Library/Perl/5.30/OpenSSL/Query.pm line 118.
 at [...]/tools/review-tools/gitaddrev line 42.

Maybe this issue is specific to MacOS.

I got this after successfully installing the Perl support as mentioned in #134 (comment).

The instructions on how to push are for public releases. We need to do something different for premium releases. The staging script should issue the correct instructions for the release type.

Dappskick![812559AF-7716-4741-809C-F4D0D1D30EBC](https://user-images.githubusercontent.com/96438570/180273524-82943d29-fc73-48a3-a4fe-8e8e17744105.png)

Originally posted by @Beckyscrypto in openssl/openssl#18829 (comment)

Addition of labels doesn't result in a notification being sent out. It would be good if we sent out a notification when an urgent label has been applied to a PR

Premium releases should not be uploaded to the server, but the release staging script does so by default. It's possible to pass a flag to stop the upload, but it ought to default to not uploaded for premium releases. Or if we are going to require a flag then we ought to document the need in the staging instructions.

The two remaining fuzzing builds should become actions.

openssl/openssl#23698

Since a couple of months it happens to me rather often
(for instance, today for openssl/openssl#20298 and openssl/openssl#20257) that
after a successful pick-to-branch the commits do not really end up (or at least do not prevail) in the upstream destination.

As a workaround, I have to check after a couple of minutes and in case the commits were silently ignored, push them manually.

It would be nice if we sent out a notification to openssl-commits when a PR becomes ready-to-merge, mentioning the reviewer names in the notification as a reminder that they might want to come back and merge the PR

Why wasn't the CLA: trivial label set earlier? AFAIK there is no automatic labelling if a commit contains "CLA: trivial" in the header.It's purely manual. Perhaps @iamamoose could work his magic...?

It seems like my addrev script stopped working a while ago. (Don't know when, haven't been using it for a while).

msp@office:~/src/openssl/master$ addrev --prnum=20106 @beldmit
Rewrite 8eaa815234dc00804c07d6ab7a1e5d7283c81df8 (1/1) (0 seconds passed, remaining 0 predicted)    Can't locate OpenSSL/Query/REST.pm in @INC (you may need to install the OpenSSL::Query::REST module) (@INC contains: /etc/perl /usr/local/lib/x86_64-linux-gnu/perl/5.32.1 /usr/local/share/perl/5.32.1 /usr/lib/x86_64-linux-gnu/perl5/5.32 /usr/share/perl5 /usr/lib/x86_64-linux-gnu/perl-base /usr/lib/x86_64-linux-gnu/perl/5.32 /usr/share/perl/5.32 /usr/local/lib/site_perl) at /home/msp/openssl/tools/review-tools/gitaddrev line 9.
BEGIN failed--compilation aborted at /home/msp/openssl/tools/review-tools/gitaddrev line 9.
msg filter failed: gitaddrev --prnum=20106 --reviewer=@abc [email protected]
 
addrev failed
Died at /home/msp/openssl/tools/review-tools/addrev line 89.

I checked that my tools directory is up-to-date

msp@office:~/src/openssl/master$ which addrev
/home/msp/openssl/tools/review-tools/addrev
msp@office:~/src/openssl/master$ cd /home/msp/openssl/tools/
msp@office:~/openssl/tools$ git log --oneline -1
ec24476 pick-to-branch: fix exit message by making variable name consistent

and the review-tools/README tells me to look for a README.md in the OpenSSL-Query folder:

The README.md however has been removed in commit 0d8b319 by @levitte.

What am I missing?

We could have the tool comment on, or just do an interesting weekly summary of PRs:

"No comments or pushes in n weeks"

Being careful to avoid any comments the tool makes when counting comments.

On git fetch from [email protected]:tools.git I get

FATAL -- ACCESS DENIED
Repo            tools                                                       
User            [email protected]                                                
Stage           Before git was called                                       
Operation       Repo read                                                   

======== This repository has been moved to [email protected]:otc/tools.git ========

FATAL: R any tools [email protected] DENIED by fallthru
(or you mis-spelled the reponame)
fatal: Could not read from remote repository.

and when I try fetching from [email protected]:otc/tools.git I get

[email protected]: Permission denied (publickey).
fatal: Could not read from remote repository.

1.1.1 is now a premium release and should use the premium email announcement template. It is still using the public announcement template.

HOWTO-publish-a-release says this about sending email announcements:

They
should be sent from the account of the person that owns the key used for
signing the release announcement

But, the email announcements are now signed by the team key (openssl-security/openssl-omc). It's unclear which user account needs to be used.

Sending from the wrong user account means that the gpg signatures will fail to verify in some email clients.

When running stage-release.sh if you neglect to supply the "--local-user" argument then it defaults to the current user. This is almost never the correct answer since it signs the release tarball with the current user's gpg key, rather than the team key.

It probably should default to the team key, or alternatively fail.

With a release commit the review rules are relaxed and the author of a commit (assuming they are a committer) is counted as a reviewer. This doesn't work with addrev --release which requires 2 reviewers excluding the author.

Proposed text:

   OpenSSL version 1.0.2zh released
   ================================

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

   The OpenSSL project team is pleased to announce the release of
   version 1.0.2zh of our open source toolkit for SSL/TLS.

   OpenSSL 1.0.2zh is available for download via HTTPS from the following
   location on our support system:

   https://github.openssl.org/openssl/extended-releases/releases/tag/OpenSSL_1_0_2zh

   If you have not yet established access to our support system server,
   please contact us on [email protected] to arrange your set up.

   The distribution file name is:

    o openssl-1.0.2zh.tar.gz
      Size: xxxxxxx
      SHA1 checksum:  xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
      SHA256 checksum:  xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx

   The checksums were calculated using the following commands:

    openssl sha1 openssl-1.0.2zh.tar.gz
    openssl sha256 openssl-1.0.2zh.tar.gz

   Yours,

   The OpenSSL Project Team.

The new rules that say the author cannot be one of the reviewers of a pull request do not apply to release commits and CHANGES/NEWS updates. But addrev doesn't know about this special case and refuses to add the appropriate headers.

It's currently changing approval: done to approval: ready to merge, but it would also be handy that it adds the approval: done automatically.