diff mbox series

[v2,1/3] package/erlang: bump to version 22.2

Message ID 20200204100733.22106-1-frank.vanbever@essensium.com
State Accepted
Headers show
Series [v2,1/3] package/erlang: bump to version 22.2 | expand

Commit Message

Frank Vanbever Feb. 4, 2020, 10:07 a.m. UTC
The issue fixed by 0003-Link-with-LDLIBS-instead-of-LIBS-for-DED.patch (ERL-529)
has been fixed since OTP20.3 (a5cbcbdb85) and is no longer required.

OTP provides wrapper scripts (otp_build and configure) which perform tasks
previously handled directly by autotools i.e. autoreconf.

Signed-off-by: Frank Vanbever <frank.vanbever@essensium.com>
---
Changes v1 -> v2:
 - nothing

My motivation for this version bump is that I want to bump RabbitMQ to the
latest version (3.8.2) which depends on Erlang 21.x or later. There are 2
packages which depend on Erlang that I tested. The test consisted of booting and
ensuring the daemon is running.

- ejabberd:
  daemon starts without issue
- RabbitMQ:
  I encountered an issue where the binaries are installed in
  rabbitmq-0.0.0 but the symlink in /usr/sbin expects them to be in
  rabbitmq-3.6.6. After manually fixing the symlink the daemon started without
  issue. The symlink issue is unrelated to Erlang and does not show up with
  3.8.2 so I did not investigate this further.

Signed-off-by: Frank Vanbever <frank.vanbever@essensium.com>
---
 ...-with-LDLIBS-instead-of-LIBS-for-DED.patch | 42 -------------------
 package/erlang/erlang.hash                    |  4 +-
 package/erlang/erlang.mk                      | 19 ++++++---
 3 files changed, 16 insertions(+), 49 deletions(-)
 delete mode 100644 package/erlang/0003-Link-with-LDLIBS-instead-of-LIBS-for-DED.patch

Comments

Thomas Petazzoni Feb. 5, 2020, 9:37 a.m. UTC | #1
Hello Frank,

Thanks for the new iteration. However, one question remains. See below.

On Tue,  4 Feb 2020 11:07:31 +0100
Frank Vanbever <frank.vanbever@essensium.com> wrote:

>  # Patched erts/aclocal.m4
> -ERLANG_AUTORECONF = YES
> +define ERLANG_RUN_AUTOCONF
> +	cd $(@D) && PATH=$(BR_PATH) ./otp_build autoconf
> +endef
> +HOST_ERLANG_DEPENDENCIES = host-autoconf
> +ERLANG_PRE_CONFIGURE_HOOKS += ERLANG_RUN_AUTOCONF
> +
> +define ERLANG_RUN_SAVE_BOOTSTRAP
> +	cd $(@D) && PATH=$(BR_PATH) ./otp_build save_bootstrap
> +endef

Why is this save_bootstrap now needed ? Looking at HOWTO/INSTALL.md in
the Erlang source code shows:

"""

#### Pre-built Source Release ####

The source release is delivered with a lot of platform independent
build results already pre-built. If you want to remove these pre-built
files, invoke `./otp_build remove_prebuilt_files` from the `$ERL_TOP`
directory. After you have done this, you can build exactly the same way
as before, but the build process will take a much longer time.

> *WARNING*: Doing `make clean` in an arbitrary directory of the source
> tree, may remove files needed for bootstrapping the build.
>
> Doing `./otp_build save_bootstrap` from the `$ERL_TOP` directory before
> doing `make clean` will ensure that it will be possible to build after
> doing `make clean`. `./otp_build save_bootstrap` will be invoked
> automatically when `make` is invoked from `$ERL_TOP` with either the
> `clean` target, or the default target. It is also automatically invoked
> if `./otp_build remove_prebuilt_files` is invoked.
>
> If you need to verify the bootstrap beam files match the provided
> source files, use `./otp_build update_primary` to create a new commit that
> contains differences, if any exist.

"""

Which doesn't really help in understanding why we need to call this
save_bootstrap action in the context of Buildroot. It definitely
warrants an explanation in the commit log and a small comment above the
code. No need to resend a new iteration of the entire patch series,
just give some more details by e-mail, I can amend the patch when
applying.

Thanks!

Thomas
Frank Vanbever Feb. 5, 2020, 11:15 a.m. UTC | #2
Hello Thomas,

On Wednesday, 5 February 2020 10:37:39 CET Thomas Petazzoni wrote:
> Why is this save_bootstrap now needed ?

During development I encountered build failures due to the following file not 
being present:

    otp_src_22.2/bootstrap/lib/kernel/include/logger.hrl

The save_bootstrap command creates this file.

I attempted to reproduce this build failure today and I am unable to do so.
Likely the behaviour I saw was an artifact of some manipulation I did to fix 
the build rather than inherent to Erlang itself.

AFAICT all lines related to ERLANG_RUN_SAVE_BOOTSTRAP can be removed as they 
serve no purpose. This is as you mention also supported by the Erlang 
documentation.

Given that it's removing code is this still something you can amend during the 
merge or do you want me to resubmit the patches?

Br,
Frank
Thomas Petazzoni Feb. 5, 2020, 2:42 p.m. UTC | #3
Hello Frank,

I've applied your patch, but with some changes. See below.

On Tue,  4 Feb 2020 11:07:31 +0100
Frank Vanbever <frank.vanbever@essensium.com> wrote:

>  # See note below when updating Erlang
> -ERLANG_VERSION = 21.0
> +ERLANG_VERSION = 22.2
>  ERLANG_SITE = http://www.erlang.org/download
>  ERLANG_SOURCE = otp_src_$(ERLANG_VERSION).tar.gz
>  ERLANG_DEPENDENCIES = host-erlang
> @@ -15,11 +15,20 @@ ERLANG_LICENSE_FILES = LICENSE.txt
>  ERLANG_INSTALL_STAGING = YES
>  
>  # Patched erts/aclocal.m4
> -ERLANG_AUTORECONF = YES
> +define ERLANG_RUN_AUTOCONF
> +	cd $(@D) && PATH=$(BR_PATH) ./otp_build autoconf
> +endef
> +HOST_ERLANG_DEPENDENCIES = host-autoconf
> +ERLANG_PRE_CONFIGURE_HOOKS += ERLANG_RUN_AUTOCONF

ERLANG_AUTORECONF = YES does the thing for both the host and target
packages.

However here, you are adding host-autoconf as a dependency for
host-erlang, but you're doing the autoconf only on the target erlang.
So I fixed that to ensure we add host-autoconf to both target/host
variants of erlang (which is pedantic since anyway erlang depends on
host-erlang) and do the autoconf on both host and target.

> +define ERLANG_RUN_SAVE_BOOTSTRAP
> +	cd $(@D) && PATH=$(BR_PATH) ./otp_build save_bootstrap
> +endef
> +ERLANG_POST_CONFIGURE_HOOKS += ERLANG_RUN_SAVE_BOOTSTRAP

As we discussed, this does not seem to be needed, at least from my
testing, so I dropped this.

Best regards,

Thomas
Johan Oudinet Feb. 10, 2020, 1:28 p.m. UTC | #4
Hi Thomas, Frank, All,

Sorry for the late reply.

On Wed, Feb 5, 2020 at 3:42 PM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> I've applied your patch, but with some changes. See below.
>
> On Tue,  4 Feb 2020 11:07:31 +0100
> Frank Vanbever <frank.vanbever@essensium.com> wrote:
>
> > +define ERLANG_RUN_SAVE_BOOTSTRAP
> > +     cd $(@D) && PATH=$(BR_PATH) ./otp_build save_bootstrap
> > +endef
> > +ERLANG_POST_CONFIGURE_HOOKS += ERLANG_RUN_SAVE_BOOTSTRAP
>
> As we discussed, this does not seem to be needed, at least from my
> testing, so I dropped this.

I've just tested it, doing a fresh compilation from buildroot/master
and I got the following error:
make[4]: Entering directory
'/home/johan/Documents/buildroot/output/build/erlang-22.2/lib/ssl/src'
make[4]: *** No rule to make target
'../../../bootstrap/lib/kernel/include/logger.hrl', needed by
'../ebin/dtls_connection.beam'.  Stop.

Since Frank said this file is created by the save_bootstrap command, I
guess it is finally required.
Thomas Petazzoni Feb. 10, 2020, 1:30 p.m. UTC | #5
On Mon, 10 Feb 2020 14:28:14 +0100
Johan Oudinet <johan.oudinet@gmail.com> wrote:

> > As we discussed, this does not seem to be needed, at least from my
> > testing, so I dropped this.  
> 
> I've just tested it, doing a fresh compilation from buildroot/master
> and I got the following error:
> make[4]: Entering directory
> '/home/johan/Documents/buildroot/output/build/erlang-22.2/lib/ssl/src'
> make[4]: *** No rule to make target
> '../../../bootstrap/lib/kernel/include/logger.hrl', needed by
> '../ebin/dtls_connection.beam'.  Stop.
> 
> Since Frank said this file is created by the save_bootstrap command, I
> guess it is finally required.

Frank indeed reported reaching the same error as the one you're seeing
here. However, last week, I also did a fresh build, and did not get
this issue. Weird.

Thomas
Johan Oudinet Feb. 10, 2020, 3:20 p.m. UTC | #6
On Mon, Feb 10, 2020 at 2:30 PM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> On Mon, 10 Feb 2020 14:28:14 +0100
> Johan Oudinet <johan.oudinet@gmail.com> wrote:
>
> > > As we discussed, this does not seem to be needed, at least from my
> > > testing, so I dropped this.
> >
> > I've just tested it, doing a fresh compilation from buildroot/master
> > and I got the following error:
> > make[4]: Entering directory
> > '/home/johan/Documents/buildroot/output/build/erlang-22.2/lib/ssl/src'
> > make[4]: *** No rule to make target
> > '../../../bootstrap/lib/kernel/include/logger.hrl', needed by
> > '../ebin/dtls_connection.beam'.  Stop.
> >
> > Since Frank said this file is created by the save_bootstrap command, I
> > guess it is finally required.
>
> Frank indeed reported reaching the same error as the one you're seeing
> here. However, last week, I also did a fresh build, and did not get
> this issue. Weird.

Well, what's strange to me is why the ssl application is looking for
logger.hrl into erlang-22.2/bootstrap/lib/kernel/include. This file is
present in erlang-22.2/lib/kernel/include.
The erlang build system is a real mess to me. If I understand
correctly HOWTO/BOOSTSTRAP.md, the bootstrap directory is supposed to
contain what they call "primary bootstrap" content, which is code
compiled from source files in the lib{kernel,stdlib,compiler}.
I don't think logger.hrl is a compiled source code, it simply defines
a bunch of macros for logging purposes. So to my understanding, the
ssl application shouldn't look for it inside the bootstrap directory.

Investigating further into this issue, I found a strange file that
references logger.hrl from the wrong location: lib/ssl/src/deps/ssl.d
This file is created by the following rule in lib/Makefile:
$(DEP_FILE): $(ERL_FILES)
        $(gen_verbose)erlc -M $(ERL_FILES) \
        | sed "s@$(ERL_TOP)@../../..@g" \
        | sed "s/\.$(EMULATOR)/\.$$\(EMULATOR\)/" \
        | sed 's@^dtls_@$$(EBIN)/dtls_@' \
        | sed 's@^inet_@$$(EBIN)/inet_@' \
        | sed 's@^ssl_@$$(EBIN)/ssl_@' \
        | sed 's@^tls_@$$(EBIN)/tls_@' \
        > $(DEP_FILE)

The idea to call erlc to generate a set of dependencies while we are
building erlang seems wrong to me... we probably need to bypass this
rule.
Hopefully, this file ssl.d is already present in the archive. So I
think a fix would be to remove bootstrap from the path to logger.hrl
via a sed command before calling make?

This might be a good idea to report this error upstream as well.
Johan Oudinet Feb. 10, 2020, 3:27 p.m. UTC | #7
On Mon, Feb 10, 2020 at 4:20 PM Johan Oudinet <johan.oudinet@gmail.com> wrote:
> Hopefully, this file ssl.d is already present in the archive. So I
> think a fix would be to remove bootstrap from the path to logger.hrl
> via a sed command before calling make?
>
> This might be a good idea to report this error upstream as well.

There is no deps directory inside lib/ssl/src in the last release of
erlang (22.2.6). I guess this was a left over from the guy who made
the archive?
So my fix proposition to simply sed this file won't work in future
erlang versions.
Johan Oudinet Feb. 10, 2020, 3:51 p.m. UTC | #8
On Mon, Feb 10, 2020 at 4:27 PM Johan Oudinet <johan.oudinet@gmail.com> wrote:
>
> On Mon, Feb 10, 2020 at 4:20 PM Johan Oudinet <johan.oudinet@gmail.com> wrote:
> > Hopefully, this file ssl.d is already present in the archive. So I
> > think a fix would be to remove bootstrap from the path to logger.hrl
> > via a sed command before calling make?
> >
> > This might be a good idea to report this error upstream as well.
>
> There is no deps directory inside lib/ssl/src in the last release of
> erlang (22.2.6). I guess this was a left over from the guy who made
> the archive?
> So my fix proposition to simply sed this file won't work in future
> erlang versions.

I confirm that removing lib/ssl/src/deps directory fix the build issue
on my machine. I can submit a patch that removes this directory as a
PRE_CONFIGURE hook, what do you think?
Thomas Petazzoni Feb. 10, 2020, 3:57 p.m. UTC | #9
On Mon, 10 Feb 2020 16:51:48 +0100
Johan Oudinet <johan.oudinet@gmail.com> wrote:

> > There is no deps directory inside lib/ssl/src in the last release of
> > erlang (22.2.6). I guess this was a left over from the guy who made
> > the archive?
> > So my fix proposition to simply sed this file won't work in future
> > erlang versions.  
> 
> I confirm that removing lib/ssl/src/deps directory fix the build issue
> on my machine. I can submit a patch that removes this directory as a
> PRE_CONFIGURE hook, what do you think?

Is this an upstream bug, i.e this folder should not be present in the
release tarball?

Thomas
Johan Oudinet Feb. 10, 2020, 4:07 p.m. UTC | #10
On Mon, Feb 10, 2020 at 4:57 PM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> On Mon, 10 Feb 2020 16:51:48 +0100
> Johan Oudinet <johan.oudinet@gmail.com> wrote:
>
> > > There is no deps directory inside lib/ssl/src in the last release of
> > > erlang (22.2.6). I guess this was a left over from the guy who made
> > > the archive?
> > > So my fix proposition to simply sed this file won't work in future
> > > erlang versions.
> >
> > I confirm that removing lib/ssl/src/deps directory fix the build issue
> > on my machine. I can submit a patch that removes this directory as a
> > PRE_CONFIGURE hook, what do you think?
>
> Is this an upstream bug, i.e this folder should not be present in the
> release tarball?

I think so, yes. This directory has been removed on the next version : 22.2.1
Well, we could also simply bump the version. As a minor release, it
should not require too much effort.
Johan Oudinet Feb. 10, 2020, 4:48 p.m. UTC | #11
On Mon, Feb 10, 2020 at 5:07 PM Johan Oudinet <johan.oudinet@gmail.com> wrote:
>
> On Mon, Feb 10, 2020 at 4:57 PM Thomas Petazzoni
> <thomas.petazzoni@bootlin.com> wrote:
> >
> > Is this an upstream bug, i.e this folder should not be present in the
> > release tarball?
>
> I think so, yes. This directory has been removed on the next version : 22.2.1
> Well, we could also simply bump the version. As a minor release, it
> should not require too much effort.

Actually, they don't put minor releases into their download section.
So I think it is best to patch it as I mentioned it. I've also opened
a bug report about it:
https://bugs.erlang.org/browse/ERL-1168

I'll submit the patch in a few minutes.
diff mbox series

Patch

diff --git a/package/erlang/0003-Link-with-LDLIBS-instead-of-LIBS-for-DED.patch b/package/erlang/0003-Link-with-LDLIBS-instead-of-LIBS-for-DED.patch
deleted file mode 100644
index ad0bb6b453..0000000000
--- a/package/erlang/0003-Link-with-LDLIBS-instead-of-LIBS-for-DED.patch
+++ /dev/null
@@ -1,42 +0,0 @@ 
-From 011752ec7b31e3dde376270fc65c7ee70644f6e7 Mon Sep 17 00:00:00 2001
-From: Johan Oudinet <johan.oudinet@gmail.com>
-Date: Wed, 6 Dec 2017 15:01:17 +0100
-Subject: [PATCH] Link with LDLIBS instead of LIBS for DED
-
-Fix ERL-529 by avoiding to link with libz for no reason.
-
-Signed-off-by: Johan Oudinet <johan.oudinet@gmail.com>
----
- lib/asn1/c_src/Makefile             | 2 +-
- lib/runtime_tools/c_src/Makefile.in | 2 +-
- 2 files changed, 2 insertions(+), 2 deletions(-)
-
-diff --git a/lib/asn1/c_src/Makefile b/lib/asn1/c_src/Makefile
-index 1f714df357..f7c6b8b9bc 100644
---- a/lib/asn1/c_src/Makefile
-+++ b/lib/asn1/c_src/Makefile
-@@ -126,7 +126,7 @@ $(NIF_LIB_FILE): $(NIF_STATIC_OBJ_FILES)
- 	$(V_RANLIB) $@
- 
- $(NIF_SHARED_OBJ_FILE): $(NIF_OBJ_FILES)
--	$(V_LD) $(LDFLAGS) -o $(NIF_SHARED_OBJ_FILE) $(NIF_OBJ_FILES) $(CLIB_FLAGS) $(LIBS)
-+	$(V_LD) $(LDFLAGS) -o $(NIF_SHARED_OBJ_FILE) $(NIF_OBJ_FILES) $(CLIB_FLAGS) $(LDLIBS)
- 
- # ----------------------------------------------------
- # Release Target
-diff --git a/lib/runtime_tools/c_src/Makefile.in b/lib/runtime_tools/c_src/Makefile.in
-index 4530a83aee..4e13e0d789 100644
---- a/lib/runtime_tools/c_src/Makefile.in
-+++ b/lib/runtime_tools/c_src/Makefile.in
-@@ -95,7 +95,7 @@ $(OBJDIR)/%$(TYPEMARKER).o: %.c dyntrace_lttng.h
- 	$(V_CC) -c -o $@ $(ALL_CFLAGS) $<
- 
- $(LIBDIR)/%$(TYPEMARKER).@DED_EXT@: $(OBJDIR)/%$(TYPEMARKER).o
--	$(V_LD) $(LDFLAGS) -o $@ $^ $(LIBS)
-+	$(V_LD) $(LDFLAGS) -o $@ $^ $(LDLIBS)
- 
- clean:
- 	rm -f $(TRACE_LIBS)
--- 
-2.14.1
-
diff --git a/package/erlang/erlang.hash b/package/erlang/erlang.hash
index 616c85e9ae..3c2f039496 100644
--- a/package/erlang/erlang.hash
+++ b/package/erlang/erlang.hash
@@ -1,4 +1,4 @@ 
 # md5 from http://www.erlang.org/download/MD5, sha256 locally computed
-md5 350988f024f88e9839c3715b35e7e27a  otp_src_21.0.tar.gz
-sha256 c7d247c0cad2d2e718eaca2e2dff051136a1347a92097abf19ebf65ea2870131  otp_src_21.0.tar.gz
+md5 b2b48dad6e69c1e882843edbf2abcfd3  otp_src_22.2.tar.gz
+sha256 89c2480cdac566065577c82704a48e10f89cf2e6ca5ab99e1cf80027784c678f  otp_src_22.2.tar.gz
 sha256 809fa1ed21450f59827d1e9aec720bbc4b687434fa22283c6cb5dd82a47ab9c0  LICENSE.txt
diff --git a/package/erlang/erlang.mk b/package/erlang/erlang.mk
index ca0aa0b770..fb26d6589f 100644
--- a/package/erlang/erlang.mk
+++ b/package/erlang/erlang.mk
@@ -5,7 +5,7 @@ 
 ################################################################################
 
 # See note below when updating Erlang
-ERLANG_VERSION = 21.0
+ERLANG_VERSION = 22.2
 ERLANG_SITE = http://www.erlang.org/download
 ERLANG_SOURCE = otp_src_$(ERLANG_VERSION).tar.gz
 ERLANG_DEPENDENCIES = host-erlang
@@ -15,11 +15,20 @@  ERLANG_LICENSE_FILES = LICENSE.txt
 ERLANG_INSTALL_STAGING = YES
 
 # Patched erts/aclocal.m4
-ERLANG_AUTORECONF = YES
+define ERLANG_RUN_AUTOCONF
+	cd $(@D) && PATH=$(BR_PATH) ./otp_build autoconf
+endef
+HOST_ERLANG_DEPENDENCIES = host-autoconf
+ERLANG_PRE_CONFIGURE_HOOKS += ERLANG_RUN_AUTOCONF
+
+define ERLANG_RUN_SAVE_BOOTSTRAP
+	cd $(@D) && PATH=$(BR_PATH) ./otp_build save_bootstrap
+endef
+ERLANG_POST_CONFIGURE_HOOKS += ERLANG_RUN_SAVE_BOOTSTRAP
 
 # Whenever updating Erlang, this value should be updated as well, to the
 # value of EI_VSN in the file lib/erl_interface/vsn.mk
-ERLANG_EI_VSN = 3.10.3
+ERLANG_EI_VSN = 3.13.1
 
 # The configure checks for these functions fail incorrectly
 ERLANG_CONF_ENV = ac_cv_func_isnan=yes ac_cv_func_isinf=yes
@@ -38,7 +47,7 @@  HOST_ERLANG_CONF_ENV += ERL_TOP=$(@D)
 
 # erlang uses openssl for all things crypto. Since the host tools (such as
 # rebar) uses crypto, we need to build host-erlang with support for openssl.
-HOST_ERLANG_DEPENDENCIES = host-openssl
+HOST_ERLANG_DEPENDENCIES += host-openssl
 HOST_ERLANG_CONF_OPTS = --without-javac --with-ssl=$(HOST_DIR)
 
 HOST_ERLANG_CONF_OPTS += --without-termcap
@@ -65,7 +74,7 @@  ERLANG_CONF_OPTS += --without-odbc
 endif
 
 # Always use Buildroot's zlib
-ERLANG_CONF_OPTS += --enable-shared-zlib
+ERLANG_CONF_OPTS += --disable-builtin-zlib
 ERLANG_DEPENDENCIES += zlib
 
 # Remove source, example, gs and wx files from staging and target.