build.info extensions: variable value substitutions and multi-item statements #11185

levitte · 2020-02-26T13:51:41Z

This started with a bug notification I got, that apps/progs.pl would fail at times. It turns out that it should be run from the build tree, not from the source tree.

This also touched an older thought, that some files (the majority of them) that we currently generate with make update and thereby have versioned don't actually need it. They can as well be generated in build time.

This PR changes things so apps/progs.c and apps/progs.h are generated at build time, and adds the necessary expansions of the build.info syntax to allow this to be done smoothly enough.

levitte · 2020-02-26T13:53:28Z

@romen, would you mind trying this with your setup?

romen · 2020-02-26T14:37:53Z

I am not capable of reviewing the Perl, but I can give feedback on testing.

Setup

mkdir -p $HOME/test_oot/b/build
cd $HOME/test_oot
git clone $OPENSSL_GIT_URL # plus some setup to have refs for github PRs
cd openssl
git co github/pr-11185
S=`pwd`
cd $HOME/test_oot/b/build
$S/config
make update
make [-j8] build_generated # I am avoiding the parallel make run at the moment for debugging
make [-j8] build_all_generated # I am avoiding the parallel make run at the moment for debugging
make doc-nits
make [-s -j8] # I am avoiding the parallel make run at the moment for debugging
make cmd-nits
make VF=1 test

Results

$SRC/config works
make update does not fail anymore with this change
make build_generated does not fail
make build_all_generated does not fail
make doc-nits does not fail
make fails with

ar: creating apps/libapps.a
ar: creating libcrypto.a
ar: creating libssl.a
ar: creating providers/libcommon.a
ar: creating providers/libfips.a
ar: creating providers/libimplementations.a
ar: creating providers/liblegacy.a
ar: creating providers/libnonfips.a
ar: creating test/libtestutil.a
../../openssl/apps/dhparam.c:23:11: fatal error: progs.h: No such file or directory
 # include "progs.h"
           ^~~~~~~~~
compilation terminated.
Makefile:21484: recipe for target 'apps/openssl-bin-dhparam.o' failed
make[1]: *** [apps/openssl-bin-dhparam.o] Error 1
Makefile:2873: recipe for target 'build_sw' failed
make: *** [build_sw] Error 2

levitte · 2020-02-26T15:14:47Z

Found the problem, fixup commit added

romen · 2020-02-26T15:48:51Z

With the latest fixup commit I can confirm I don't get failures in my setup, but Travis failures seem relevant!

richsalz · 2020-02-26T15:50:51Z

don't you need to put the removed files into .gitignore?

romen · 2020-02-26T15:59:17Z

@levitte I also noticed a

grep: ./util/../include/openssl/opensslv.h: No such file or directory

message that seems to be triggered by $ if [ -n "$DESTDIR" ]; then sh .travis-create-release.sh $TRAVIS_OS_NAME; tar -xzf _srcdist.tar.gz; mkdir _build; cd _build; srcdir=../_srcdist; top=..; else srcdir=.; top=.; fi

it does not cause a failure, but maybe it is worth investigating?

t8m · 2020-02-26T16:07:36Z

You could drop the NOUPDATE variable support from .travis.yml that I've added in #11181 to workaround the progs update with no-deprecated.

levitte · 2020-02-26T17:30:00Z

grep: ./util/../include/openssl/opensslv.h: No such file or directory

Is this new? Surprising, that should have happened long ago. util/mktar.sh in master needs a bit of adaptation, it should read the VERSION file. I'll get to that tomorrow

romen · 2020-02-26T17:35:55Z

grep: ./util/../include/openssl/opensslv.h: No such file or directory
Is this new? Surprising, that should have happened long ago. util/mktar.sh in master needs a bit of adaptation, it should read the VERSION file. I'll get to that tomorrow

There is a high probability that this has been there for a long time now, but we don't notice because it does not trigger an error and is hidden in the travis output!

levitte · 2020-02-27T01:05:37Z

util/mktar.sh fixed in #11190. The error doesn't impact Travis because we give the tarball and content prefix a name that's not derived on the version, so it's not relevant here.

levitte · 2020-02-27T01:09:12Z

You could drop the NOUPDATE variable support from .travis.yml that I've added in #11181 to workaround the progs update with no-deprecated.

Done.

romen · 2020-02-27T05:44:42Z

Still failing in Travis with

$ if ! $make2; then echo -e '\052\052 FAILED -- MAKE'; travis_terminate 1; fi;
ar: creating apps/libapps.a
ar: creating libcrypto.a
ar: creating libssl.a
ar: creating providers/libcommon.a
ar: creating providers/libfips.a
ar: creating providers/libimplementations.a
ar: creating providers/liblegacy.a
ar: creating providers/libnonfips.a
ar: creating test/libtestutil.a
Couldn't open apps/progs.c: No such file or directory
Makefile:21421: recipe for target 'apps/progs.h' failed
make[1]: *** [apps/progs.h] Error 2
Makefile:2862: recipe for target 'build_sw' failed
make: *** [build_sw] Error 2
** FAILED -- MAKE

levitte · 2020-02-27T07:45:45Z

Aha, apps/progs.c must be made before apps/progs.h... of course.
Fixed!

Configurations/unix-Makefile.tmpl

t8m

The openssl build system is becoming so full-featured that it could be eventually shipped as a separate product. 😄

doc/internal/man7/build.info.pod

levitte · 2020-02-27T11:41:21Z

The openssl build system is becoming so full-featured that it could be eventually shipped as a separate product. 😄

Hush, don't give my secret plans away 😉

romen · 2020-02-27T11:45:52Z

The openssl build system is becoming so full-featured that it could be eventually shipped as a separate product. smile

Hush, don't give my secret plans away

EVP_Build?

romen · 2020-02-27T13:42:47Z

I just manually restarted the failing (timeout) travis job

richsalz · 2020-02-27T13:43:02Z

I know there are smiley faces here, but I think the project should be concerned about the amount of Perl knowledge necessary to change things. When Andy left it took a very long time to partially recover perlasm knowledge.

levitte · 2020-02-28T06:50:24Z

All CIs are go

t8m

Approving although it would be nice to have another review of the perl changes by someone with more perl skills.

openssl-machine · 2020-02-29T09:00:17Z

This pull request is ready to merge

So far, the "index" part of KEYWORD[whatever] could only handle one item. There are cases, however, where we want to add the exact same value to multiple items. This is especially helpful if a variable that may have multi-item values are used in the "index" part. Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org> (Merged from openssl#11185)

Use case: having a variable with multiple source files in its value, and wanting to refer to the corresponding object file. $SRCS=foo.c bar.c SOURCE[program]=$SRCS DEPEND[${SRCS/.c/.o}]=prog.h GENERATE[prog.h]=... Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org> (Merged from openssl#11185)

util/progs.pl depends on the build tree (on configdata.pm, specifically), so it needs to be run from the build tree. But why stop there? We might as well generate apps/progs.c and apps/progs.h when building. Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org> (Merged from openssl#11185)

There were some remaining old code and comments that don't serve a purpose any longer. Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org> (Merged from openssl#11185)

Since they are generated in build time, there's not need to keep them in the source tree. Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org> (Merged from openssl#11185)

It was a temporary measure to deal with the fact that util/progs.pl didn't work right at all times, but that has now been fixed. Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org> (Merged from openssl#11185)

Change relative nesting depth of the OOT build_dir Build failures seem to depend on what is the relative nesting level between $srcdir and $blddir. This seems potentially related to: - openssl#11184 - openssl#11185

levitte added branch: master Applies to master branch approval: review pending This pull request needs review by a committer labels Feb 26, 2020

levitte added this to the 3.0.0 milestone Feb 26, 2020

This was referenced Feb 26, 2020

Run apps/progs.pl from the build tree #11184

Closed

.travis.yml: where it matters, have build and source nesting levels differ #11186

Merged

levitte force-pushed the build.info-fixups-20200226 branch from edb2cd4 to 7346d94 Compare February 27, 2020 01:09

romen reviewed Feb 27, 2020

View reviewed changes

Configurations/unix-Makefile.tmpl Outdated Show resolved Hide resolved

t8m reviewed Feb 27, 2020

View reviewed changes

doc/internal/man7/build.info.pod Outdated Show resolved Hide resolved

t8m approved these changes Feb 28, 2020

View reviewed changes

t8m added approval: done This pull request has the required number of approvals and removed approval: review pending This pull request needs review by a committer labels Feb 28, 2020

openssl-machine removed the approval: done This pull request has the required number of approvals label Feb 29, 2020

openssl-machine added the approval: ready to merge The 24 hour grace period has passed, ready to merge label Feb 29, 2020

levitte added 6 commits March 2, 2020 03:34

Configure: Diverse cleanups

19cf440

There were some remaining old code and comments that don't serve a purpose any longer. Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org> (Merged from openssl#11185)

Remove apps/progs.c and apps/progs.h

fe909ee

Since they are generated in build time, there's not need to keep them in the source tree. Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org> (Merged from openssl#11185)

.travis.yml: Remove NOUPDATE support

97ace6c

It was a temporary measure to deal with the fact that util/progs.pl didn't work right at all times, but that has now been fixed. Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org> (Merged from openssl#11185)

levitte force-pushed the build.info-fixups-20200226 branch from b3f8f67 to 97ace6c Compare March 2, 2020 02:36

openssl-machine merged commit 97ace6c into openssl:master Mar 2, 2020

levitte deleted the build.info-fixups-20200226 branch March 2, 2020 02:37

romen mentioned this pull request May 24, 2020

[1.1.1] Out-of-tree builds fail on make update #11940

Closed

Uh oh!

build.info extensions: variable value substitutions and multi-item statements #11185

build.info extensions: variable value substitutions and multi-item statements #11185

Uh oh!

Conversation

levitte commented Feb 26, 2020

Uh oh!

levitte commented Feb 26, 2020

Uh oh!

romen commented Feb 26, 2020

Setup

Results

Uh oh!

levitte commented Feb 26, 2020

Uh oh!

romen commented Feb 26, 2020

Uh oh!

richsalz commented Feb 26, 2020

Uh oh!

romen commented Feb 26, 2020

Uh oh!

t8m commented Feb 26, 2020

Uh oh!

levitte commented Feb 26, 2020

Uh oh!

romen commented Feb 26, 2020

Uh oh!

levitte commented Feb 27, 2020

Uh oh!

levitte commented Feb 27, 2020

Uh oh!

romen commented Feb 27, 2020

Uh oh!

levitte commented Feb 27, 2020

Uh oh!

Uh oh!

t8m left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

levitte commented Feb 27, 2020

Uh oh!

romen commented Feb 27, 2020

Uh oh!

romen commented Feb 27, 2020

Uh oh!

richsalz commented Feb 27, 2020

Uh oh!

levitte commented Feb 28, 2020

Uh oh!

t8m left a comment

Choose a reason for hiding this comment

Uh oh!

openssl-machine commented Feb 29, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants