* [Buildroot] [PATCH v2 1/2] package/postgresql/postgresql.service: set locale for initdb to C
@ 2023-11-02 18:41 Adam Duskett
2023-11-02 18:41 ` [Buildroot] [PATCH v2 2/2] support/testing/tests/package/test_postgresql.py: new test Adam Duskett
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Adam Duskett @ 2023-11-02 18:41 UTC (permalink / raw)
To: buildroot; +Cc: Adam Duskett, Peter Seiderer
From: Peter Seiderer <ps.report@gmx.net>
Systemd creates a /etc/locale.conf file with LANG="C.UTF-8". On boot, systemd
reads /etc/locale.conf and sets the LANG environment variable,
(see the locale_context_load_conf method in local-setup.c.)
When initdb.c is called, a check for the LANG environment variable is called,
and if it is set to something other than "C" initdb attempts to load the
corresponding LC_CTYPE file in /usr/lib/locale/. IE: If LANG is set to C.UTF-8,
then initdb.c attempts to load /usr/lib/locale/C.UTF-8/LC_CTYPE. However, these
files do not exist on a Buildroot system, and as such, initdb throws the
following error on startup:
```
initdb: error: invalid locale settings; check LANG and LC_* environment variables
pg_ctl: database system initialization failed
```
To fix this issue, add "Environment=LANG=C" to the package provided
postgresql.service file to force Postgresql to use the C locale.
Tested-by: Adam Duskett <adam.duskett@amarulasolutions.com>
Signed-off-by: Adam Duskett <adam.duskett@amarulasolutions.com>
---
v1 -> v2:
- Get to the root cause of the problem and provide a better explination of
what is happening.
- Use Environment=LANG=C isntead of -o --locale=C
package/postgresql/postgresql.service | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/package/postgresql/postgresql.service b/package/postgresql/postgresql.service
index 539eea8964..c470c7181e 100644
--- a/package/postgresql/postgresql.service
+++ b/package/postgresql/postgresql.service
@@ -16,6 +16,10 @@ StandardOutput=syslog
StandardError=syslog
SyslogIdentifier=postgres
+# Overwrite the LANG variable to prevent systemd from passing the LANG
+# environment variable set in /etc/locale.conf.
+Environment=LANG=C
+
ExecStartPre=/bin/sh -c "if [ ! -f /var/lib/pgsql/PG_VERSION ]; then /usr/bin/pg_ctl initdb -D /var/lib/pgsql; fi"
ExecStart=/usr/bin/postgres -D /var/lib/pgsql
ExecReload=/usr/bin/kill -HUP $MAINPID
--
2.41.0
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Buildroot] [PATCH v2 2/2] support/testing/tests/package/test_postgresql.py: new test
2023-11-02 18:41 [Buildroot] [PATCH v2 1/2] package/postgresql/postgresql.service: set locale for initdb to C Adam Duskett
@ 2023-11-02 18:41 ` Adam Duskett
2023-12-18 17:31 ` Yann E. MORIN
2023-11-05 10:07 ` [Buildroot] [PATCH v2 1/2] package/postgresql/postgresql.service: set locale for initdb to C Peter Seiderer
2023-12-18 17:22 ` Yann E. MORIN
2 siblings, 1 reply; 14+ messages in thread
From: Adam Duskett @ 2023-11-02 18:41 UTC (permalink / raw)
To: buildroot; +Cc: Adam Duskett
Perform a basic check that performs the following:
- Check if /var/lib/pgsql/postmaster.pid exists.
- Check if 'psql -c SHOW server_version;' returns sucessfully.
Note: systemd takes quite a while to start up, so check the output of
`systemctl is-active postgresql` until it shows "active" with a timeout
of 15 seconds.
Signed-off-by: Adam Duskett <adam.duskett@amarulasolutions.com>
---
v1 -> v2:
- Drop ": str" in front of config (Thomas)
- Drop BR2_PER_PACKAGE_DIRECTORIES (Thomas)
- Rename TestPostgreSQLInitd to TestPostgreSQLInitSysV (Thomas)
- Use BASIC_TOOLCHAIN_CONFIG for TestPostgreSQLInitSysV (Thomas)
- Move self.assertRunOk("ls /var/lib/pgsql/postmaster.pid") to after
`systemctl is-active postgresql` returns successfully. (Yann)
- Add an explination as to why the systemd test is not using
BASIC_TOOLCHAIN_CONFIG.
DEVELOPERS | 1 +
.../testing/tests/package/test_postgresql.py | 73 +++++++++++++++++++
2 files changed, 74 insertions(+)
create mode 100644 support/testing/tests/package/test_postgresql.py
diff --git a/DEVELOPERS b/DEVELOPERS
index 26b25edd98..548cf25646 100644
--- a/DEVELOPERS
+++ b/DEVELOPERS
@@ -38,6 +38,7 @@ F: package/flutter-gallery/
F: package/flutter-pi/
F: package/flutter-sdk-bin/
F: support/testing/tests/package/test_flutter.py
+F: support/testing/tests/package/test_postgresql.py
N: Adam Heinrich <adam@adamh.cz>
F: package/jack1/
diff --git a/support/testing/tests/package/test_postgresql.py b/support/testing/tests/package/test_postgresql.py
new file mode 100644
index 0000000000..fa692bab7b
--- /dev/null
+++ b/support/testing/tests/package/test_postgresql.py
@@ -0,0 +1,73 @@
+import os
+import time
+import infra.basetest
+
+
+class TestPostgreSQLInitSysV(infra.basetest.BRTest):
+ config = infra.basetest.BASIC_TOOLCHAIN_CONFIG + \
+ """
+ BR2_PACKAGE_POSTGRESQL=y
+ BR2_TARGET_ROOTFS_EXT2=y
+ BR2_TARGET_ROOTFS_EXT2_4=y
+ BR2_TARGET_ROOTFS_EXT2_SIZE="128M"
+ # BR2_TARGET_ROOTFS_TAR is not set
+ """
+
+ def test_run(self):
+ img = os.path.join(self.builddir, "images", "rootfs.ext2")
+ self.emulator.boot(
+ arch="armv5",
+ kernel="builtin",
+ options=["-drive", f"file={img},if=scsi,format=raw"],
+ kernel_cmdline=["root=/dev/sda"])
+ self.emulator.login()
+
+ # Check if the Daemon is running
+ self.assertRunOk("ls /var/lib/pgsql/postmaster.pid")
+
+ # Check if we can connect to the database.
+ self.assertRunOk("su postgres -c \"psql -c 'SHOW server_version;'\"")
+
+
+class TestPostgreSQLInitSystemd(infra.basetest.BRTest):
+ # Taken from test_systemd.py
+ config = """
+ BR2_arm=y
+ BR2_cortex_a9=y
+ BR2_ARM_ENABLE_VFP=y
+ BR2_TOOLCHAIN_EXTERNAL=y
+ BR2_TOOLCHAIN_EXTERNAL_BOOTLIN=y
+ BR2_INIT_SYSTEMD=y
+ BR2_TARGET_GENERIC_GETTY_PORT="ttyAMA0"
+ BR2_PACKAGE_POSTGRESQL=y
+ BR2_TARGET_ROOTFS_EXT2=y
+ BR2_TARGET_ROOTFS_EXT2_4=y
+ BR2_TARGET_ROOTFS_EXT2_SIZE="128M"
+ # BR2_TARGET_ROOTFS_TAR is not set
+ """
+
+ def test_run(self):
+ img = os.path.join(self.builddir, "images", "rootfs.ext2")
+ self.emulator.boot(
+ arch="armv7",
+ kernel="builtin",
+ options=["-drive", f"file={img},if=sd,format=raw"],
+ kernel_cmdline=["root=/dev/mmcblk0"])
+ self.emulator.login()
+
+ # It may take some time for PostgreSQL to finish startup. Give it at least 15 seconds.
+ is_active = False
+ for i in range(15):
+ output, _ = self.emulator.run("systemctl is-active postgresql")
+ if output[0] == "active":
+ is_active = True
+ break
+ time.sleep(1)
+ if not is_active:
+ self.fail("postgresql failed to activate!")
+
+ # Check if the Daemon is running
+ self.assertRunOk("ls /var/lib/pgsql/postmaster.pid")
+
+ # Check if we can connect to the database.
+ self.assertRunOk("cd / && su postgres -c \"psql -c 'SHOW server_version;'\"")
--
2.41.0
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [Buildroot] [PATCH v2 1/2] package/postgresql/postgresql.service: set locale for initdb to C
2023-11-02 18:41 [Buildroot] [PATCH v2 1/2] package/postgresql/postgresql.service: set locale for initdb to C Adam Duskett
2023-11-02 18:41 ` [Buildroot] [PATCH v2 2/2] support/testing/tests/package/test_postgresql.py: new test Adam Duskett
@ 2023-11-05 10:07 ` Peter Seiderer
2023-12-18 17:22 ` Yann E. MORIN
2 siblings, 0 replies; 14+ messages in thread
From: Peter Seiderer @ 2023-11-05 10:07 UTC (permalink / raw)
To: Adam Duskett; +Cc: buildroot
Hello *,
On Thu, 2 Nov 2023 12:41:53 -0600, Adam Duskett <adam.duskett@amarulasolutions.com> wrote:
> From: Peter Seiderer <ps.report@gmx.net>
>
> Systemd creates a /etc/locale.conf file with LANG="C.UTF-8". On boot, systemd
> reads /etc/locale.conf and sets the LANG environment variable,
> (see the locale_context_load_conf method in local-setup.c.)
>
> When initdb.c is called, a check for the LANG environment variable is called,
> and if it is set to something other than "C" initdb attempts to load the
> corresponding LC_CTYPE file in /usr/lib/locale/. IE: If LANG is set to C.UTF-8,
> then initdb.c attempts to load /usr/lib/locale/C.UTF-8/LC_CTYPE. However, these
> files do not exist on a Buildroot system, and as such, initdb throws the
> following error on startup:
>
> ```
> initdb: error: invalid locale settings; check LANG and LC_* environment variables
> pg_ctl: database system initialization failed
> ```
>
> To fix this issue, add "Environment=LANG=C" to the package provided
> postgresql.service file to force Postgresql to use the C locale.
>
> Tested-by: Adam Duskett <adam.duskett@amarulasolutions.com>
> Signed-off-by: Adam Duskett <adam.duskett@amarulasolutions.com>
> ---
> v1 -> v2:
> - Get to the root cause of the problem and provide a better explination of
> what is happening.
>
> - Use Environment=LANG=C isntead of -o --locale=C
>
> package/postgresql/postgresql.service | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/package/postgresql/postgresql.service b/package/postgresql/postgresql.service
> index 539eea8964..c470c7181e 100644
> --- a/package/postgresql/postgresql.service
> +++ b/package/postgresql/postgresql.service
> @@ -16,6 +16,10 @@ StandardOutput=syslog
> StandardError=syslog
> SyslogIdentifier=postgres
>
> +# Overwrite the LANG variable to prevent systemd from passing the LANG
> +# environment variable set in /etc/locale.conf.
> +Environment=LANG=C
> +
> ExecStartPre=/bin/sh -c "if [ ! -f /var/lib/pgsql/PG_VERSION ]; then /usr/bin/pg_ctl initdb -D /var/lib/pgsql; fi"
> ExecStart=/usr/bin/postgres -D /var/lib/pgsql
> ExecReload=/usr/bin/kill -HUP $MAINPID
And for the record the links to the original patch series/review comments:
https://patchwork.ozlabs.org/project/buildroot/patch/20200920150659.7562-1-ps.report@gmx.net/
https://patchwork.ozlabs.org/project/buildroot/patch/20200920150659.7562-2-ps.report@gmx.net/
https://patchwork.ozlabs.org/project/buildroot/patch/20200920150659.7562-3-ps.report@gmx.net/
Regards,
Peter
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Buildroot] [PATCH v2 1/2] package/postgresql/postgresql.service: set locale for initdb to C
2023-11-02 18:41 [Buildroot] [PATCH v2 1/2] package/postgresql/postgresql.service: set locale for initdb to C Adam Duskett
2023-11-02 18:41 ` [Buildroot] [PATCH v2 2/2] support/testing/tests/package/test_postgresql.py: new test Adam Duskett
2023-11-05 10:07 ` [Buildroot] [PATCH v2 1/2] package/postgresql/postgresql.service: set locale for initdb to C Peter Seiderer
@ 2023-12-18 17:22 ` Yann E. MORIN
2023-12-18 17:28 ` Adam Duskett
2 siblings, 1 reply; 14+ messages in thread
From: Yann E. MORIN @ 2023-12-18 17:22 UTC (permalink / raw)
To: Adam Duskett; +Cc: Peter Seiderer, buildroot
Adam, Peter, All,
On 2023-11-02 12:41 -0600, Adam Duskett spake thusly:
> From: Peter Seiderer <ps.report@gmx.net>
So, this patch is "From Peter", but...
> Systemd creates a /etc/locale.conf file with LANG="C.UTF-8". On boot, systemd
> reads /etc/locale.conf and sets the LANG environment variable,
> (see the locale_context_load_conf method in local-setup.c.)
>
> When initdb.c is called, a check for the LANG environment variable is called,
> and if it is set to something other than "C" initdb attempts to load the
> corresponding LC_CTYPE file in /usr/lib/locale/. IE: If LANG is set to C.UTF-8,
> then initdb.c attempts to load /usr/lib/locale/C.UTF-8/LC_CTYPE. However, these
> files do not exist on a Buildroot system, and as such, initdb throws the
> following error on startup:
>
> ```
> initdb: error: invalid locale settings; check LANG and LC_* environment variables
> pg_ctl: database system initialization failed
> ```
>
> To fix this issue, add "Environment=LANG=C" to the package provided
> postgresql.service file to force Postgresql to use the C locale.
>
> Tested-by: Adam Duskett <adam.duskett@amarulasolutions.com>
> Signed-off-by: Adam Duskett <adam.duskett@amarulasolutions.com>
... the first SoB is by Adam, and there is no SoB be Peter.
In this case, I think I understand that Adam took Peter's patch, and
totally rewrote it with an alternate solution. In such a case, the
customs is to change the authorship to the new author, and keep a
Co-developed-by tag to keep credits to the first author.
So, if you can just reply to this mail stating I got things right, I can
fix authorship and tags when applying. If I got things wrong, then
please resubmit with proper tags and/or authorship.
Also, it does not make much sense that the submitter of a patch adds
their own Tested-By tag, as it is actually expected that the submitter
did test what they sent...
Thanks! :-)
Regards,
Yann E. MORIN.
> ---
> v1 -> v2:
> - Get to the root cause of the problem and provide a better explination of
> what is happening.
>
> - Use Environment=LANG=C isntead of -o --locale=C
>
> package/postgresql/postgresql.service | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/package/postgresql/postgresql.service b/package/postgresql/postgresql.service
> index 539eea8964..c470c7181e 100644
> --- a/package/postgresql/postgresql.service
> +++ b/package/postgresql/postgresql.service
> @@ -16,6 +16,10 @@ StandardOutput=syslog
> StandardError=syslog
> SyslogIdentifier=postgres
>
> +# Overwrite the LANG variable to prevent systemd from passing the LANG
> +# environment variable set in /etc/locale.conf.
> +Environment=LANG=C
> +
> ExecStartPre=/bin/sh -c "if [ ! -f /var/lib/pgsql/PG_VERSION ]; then /usr/bin/pg_ctl initdb -D /var/lib/pgsql; fi"
> ExecStart=/usr/bin/postgres -D /var/lib/pgsql
> ExecReload=/usr/bin/kill -HUP $MAINPID
> --
> 2.41.0
>
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot
--
.-----------------.--------------------.------------------.--------------------.
| Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ |
| +33 561 099 427 `------------.-------: X AGAINST | \e/ There is no |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Buildroot] [PATCH v2 1/2] package/postgresql/postgresql.service: set locale for initdb to C
2023-12-18 17:22 ` Yann E. MORIN
@ 2023-12-18 17:28 ` Adam Duskett
2023-12-18 17:39 ` Yann E. MORIN
0 siblings, 1 reply; 14+ messages in thread
From: Adam Duskett @ 2023-12-18 17:28 UTC (permalink / raw)
To: Yann E. MORIN; +Cc: Peter Seiderer, buildroot
[-- Attachment #1.1: Type: text/plain, Size: 4450 bytes --]
Yann;
I didn't add Peter as the patch is completely different than what he sent
along with a more
comprehensive explanation as to what is happening. If you want to add Peter
as a
co-author that is more than fine by me.
Adam Duskett
Senior Embedded Systems Developer
M. +1208-515-8102
adam.duskett@amarulasolutions.com
__________________________________
Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9170
info@amarulasolutions.com
www.amarulasolutions.com
On Mon, Dec 18, 2023 at 10:22 AM Yann E. MORIN <yann.morin.1998@free.fr>
wrote:
> Adam, Peter, All,
>
> On 2023-11-02 12:41 -0600, Adam Duskett spake thusly:
> > From: Peter Seiderer <ps.report@gmx.net>
>
> So, this patch is "From Peter", but...
>
> > Systemd creates a /etc/locale.conf file with LANG="C.UTF-8". On boot,
> systemd
> > reads /etc/locale.conf and sets the LANG environment variable,
> > (see the locale_context_load_conf method in local-setup.c.)
> >
> > When initdb.c is called, a check for the LANG environment variable is
> called,
> > and if it is set to something other than "C" initdb attempts to load the
> > corresponding LC_CTYPE file in /usr/lib/locale/. IE: If LANG is set to
> C.UTF-8,
> > then initdb.c attempts to load /usr/lib/locale/C.UTF-8/LC_CTYPE.
> However, these
> > files do not exist on a Buildroot system, and as such, initdb throws the
> > following error on startup:
> >
> > ```
> > initdb: error: invalid locale settings; check LANG and LC_* environment
> variables
> > pg_ctl: database system initialization failed
> > ```
> >
> > To fix this issue, add "Environment=LANG=C" to the package provided
> > postgresql.service file to force Postgresql to use the C locale.
> >
> > Tested-by: Adam Duskett <adam.duskett@amarulasolutions.com>
> > Signed-off-by: Adam Duskett <adam.duskett@amarulasolutions.com>
>
> ... the first SoB is by Adam, and there is no SoB be Peter.
>
> In this case, I think I understand that Adam took Peter's patch, and
> totally rewrote it with an alternate solution. In such a case, the
> customs is to change the authorship to the new author, and keep a
> Co-developed-by tag to keep credits to the first author.
>
> So, if you can just reply to this mail stating I got things right, I can
> fix authorship and tags when applying. If I got things wrong, then
> please resubmit with proper tags and/or authorship.
>
> Also, it does not make much sense that the submitter of a patch adds
> their own Tested-By tag, as it is actually expected that the submitter
> did test what they sent...
>
> Thanks! :-)
>
> Regards,
> Yann E. MORIN.
>
> > ---
> > v1 -> v2:
> > - Get to the root cause of the problem and provide a better
> explination of
> > what is happening.
> >
> > - Use Environment=LANG=C isntead of -o --locale=C
> >
> > package/postgresql/postgresql.service | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/package/postgresql/postgresql.service
> b/package/postgresql/postgresql.service
> > index 539eea8964..c470c7181e 100644
> > --- a/package/postgresql/postgresql.service
> > +++ b/package/postgresql/postgresql.service
> > @@ -16,6 +16,10 @@ StandardOutput=syslog
> > StandardError=syslog
> > SyslogIdentifier=postgres
> >
> > +# Overwrite the LANG variable to prevent systemd from passing the LANG
> > +# environment variable set in /etc/locale.conf.
> > +Environment=LANG=C
> > +
> > ExecStartPre=/bin/sh -c "if [ ! -f /var/lib/pgsql/PG_VERSION ]; then
> /usr/bin/pg_ctl initdb -D /var/lib/pgsql; fi"
> > ExecStart=/usr/bin/postgres -D /var/lib/pgsql
> > ExecReload=/usr/bin/kill -HUP $MAINPID
> > --
> > 2.41.0
> >
> > _______________________________________________
> > buildroot mailing list
> > buildroot@buildroot.org
> > https://lists.buildroot.org/mailman/listinfo/buildroot
>
> --
>
> .-----------------.--------------------.------------------.--------------------.
> | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics'
> conspiracy: |
> | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___
> |
> | +33 561 099 427 `------------.-------: X AGAINST | \e/ There is
> no |
> | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v
> conspiracy. |
>
> '------------------------------^-------^------------------^--------------------'
>
[-- Attachment #1.2: Type: text/html, Size: 8847 bytes --]
[-- Attachment #2: Type: text/plain, Size: 150 bytes --]
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Buildroot] [PATCH v2 2/2] support/testing/tests/package/test_postgresql.py: new test
2023-11-02 18:41 ` [Buildroot] [PATCH v2 2/2] support/testing/tests/package/test_postgresql.py: new test Adam Duskett
@ 2023-12-18 17:31 ` Yann E. MORIN
2023-12-18 17:41 ` Adam Duskett
0 siblings, 1 reply; 14+ messages in thread
From: Yann E. MORIN @ 2023-12-18 17:31 UTC (permalink / raw)
To: Adam Duskett; +Cc: buildroot
Adam, All,
On 2023-11-02 12:41 -0600, Adam Duskett spake thusly:
> Perform a basic check that performs the following:
> - Check if /var/lib/pgsql/postmaster.pid exists.
> - Check if 'psql -c SHOW server_version;' returns sucessfully.
Thanks for this new runtime test!
> Note: systemd takes quite a while to start up, so check the output of
> `systemctl is-active postgresql` until it shows "active" with a timeout
> of 15 seconds.
I think that we already concluded that testing if a unit was active or
not was not representing whether the service was actually running. In
the fultter test case for example, the unit was active, but the flutter
app was just keeping crashing again and again, so the test wsa failing
to detect failure...
So, I don't think usingt is-active is a good way to test whether a
service is actually running.
Furthermore, the unit may be active, but the service might not yet be
ready to serve, so again I don;t think it is still valid to reply on it.
> Signed-off-by: Adam Duskett <adam.duskett@amarulasolutions.com>
> ---
> - Drop BR2_PER_PACKAGE_DIRECTORIES (Thomas)
Actually, I disagree with Thomas here: we added support for PPD in the
infra, so that if PPD is enabled in a test, then TLPB is done to speed
up the test.
[--SNIP--]
> diff --git a/support/testing/tests/package/test_postgresql.py b/support/testing/tests/package/test_postgresql.py
> new file mode 100644
> index 0000000000..fa692bab7b
> --- /dev/null
> +++ b/support/testing/tests/package/test_postgresql.py
> @@ -0,0 +1,73 @@
> +import os
> +import time
> +import infra.basetest
> +
> +
> +class TestPostgreSQLInitSysV(infra.basetest.BRTest):
> + config = infra.basetest.BASIC_TOOLCHAIN_CONFIG + \
> + """
> + BR2_PACKAGE_POSTGRESQL=y
> + BR2_TARGET_ROOTFS_EXT2=y
> + BR2_TARGET_ROOTFS_EXT2_4=y
> + BR2_TARGET_ROOTFS_EXT2_SIZE="128M"
> + # BR2_TARGET_ROOTFS_TAR is not set
> + """
> +
> + def test_run(self):
> + img = os.path.join(self.builddir, "images", "rootfs.ext2")
> + self.emulator.boot(
> + arch="armv5",
> + kernel="builtin",
> + options=["-drive", f"file={img},if=scsi,format=raw"],
> + kernel_cmdline=["root=/dev/sda"])
> + self.emulator.login()
> +
> + # Check if the Daemon is running
> + self.assertRunOk("ls /var/lib/pgsql/postmaster.pid")
> + # Check if we can connect to the database.
> + self.assertRunOk("su postgres -c \"psql -c 'SHOW server_version;'\"")
I don't understand why we need to test for the PID file before we
connect to the daemon. If the daemon is not running, we won't be able to
connect to it, so the test will fail, so the PID file test is redundant,
no?
Regards,
Yann E. MORIN.
> +
> +class TestPostgreSQLInitSystemd(infra.basetest.BRTest):
> + # Taken from test_systemd.py
> + config = """
> + BR2_arm=y
> + BR2_cortex_a9=y
> + BR2_ARM_ENABLE_VFP=y
> + BR2_TOOLCHAIN_EXTERNAL=y
> + BR2_TOOLCHAIN_EXTERNAL_BOOTLIN=y
> + BR2_INIT_SYSTEMD=y
> + BR2_TARGET_GENERIC_GETTY_PORT="ttyAMA0"
> + BR2_PACKAGE_POSTGRESQL=y
> + BR2_TARGET_ROOTFS_EXT2=y
> + BR2_TARGET_ROOTFS_EXT2_4=y
> + BR2_TARGET_ROOTFS_EXT2_SIZE="128M"
> + # BR2_TARGET_ROOTFS_TAR is not set
> + """
> +
> + def test_run(self):
> + img = os.path.join(self.builddir, "images", "rootfs.ext2")
> + self.emulator.boot(
> + arch="armv7",
> + kernel="builtin",
> + options=["-drive", f"file={img},if=sd,format=raw"],
> + kernel_cmdline=["root=/dev/mmcblk0"])
> + self.emulator.login()
> +
> + # It may take some time for PostgreSQL to finish startup. Give it at least 15 seconds.
> + is_active = False
> + for i in range(15):
> + output, _ = self.emulator.run("systemctl is-active postgresql")
> + if output[0] == "active":
> + is_active = True
> + break
> + time.sleep(1)
> + if not is_active:
> + self.fail("postgresql failed to activate!")
> +
> + # Check if the Daemon is running
> + self.assertRunOk("ls /var/lib/pgsql/postmaster.pid")
> +
> + # Check if we can connect to the database.
> + self.assertRunOk("cd / && su postgres -c \"psql -c 'SHOW server_version;'\"")
> --
> 2.41.0
>
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot
--
.-----------------.--------------------.------------------.--------------------.
| Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ |
| +33 561 099 427 `------------.-------: X AGAINST | \e/ There is no |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Buildroot] [PATCH v2 1/2] package/postgresql/postgresql.service: set locale for initdb to C
2023-12-18 17:28 ` Adam Duskett
@ 2023-12-18 17:39 ` Yann E. MORIN
2023-12-18 17:44 ` Adam Duskett
0 siblings, 1 reply; 14+ messages in thread
From: Yann E. MORIN @ 2023-12-18 17:39 UTC (permalink / raw)
To: Adam Duskett; +Cc: Peter Seiderer, buildroot
Adam, All,
[Please, do not top-post]
On 2023-12-18 10:28 -0700, Adam Duskett spake thusly:
> I didn't add Peter as the patch is completely different than what he sent
> along with a more
> comprehensive explanation as to what is happening.
Then you should also have dropped him as the author; note how the first
line of the patch was still:
From: Peter Seiderer <ps.report@gmx.net>
... which git uses as the author of the patch.
> If you want to add Peter
> as a
> co-author that is more than fine by me.
It is not about what I want, it is about keeping auhtorship and SoB in
sync and coherent with who actually did what.
So if I understand correctly: you are the author, so you have your SoB,
and Peter is a co-dev for credits, right: Yes/No ? ;-)
If yes, then can I fix the authorship and add the Co-developed-by tag
for Peter: Yes/No?
If no to either, can you respin a fixed-up patch, please?
Regards,
Yann E. MORIN.
> Adam Duskett
>
> Senior Embedded Systems Developer
>
> M. +1208-515-8102
>
> adam.duskett@amarulasolutions.com
>
> __________________________________
>
>
> Amarula Solutions BV
>
> Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
>
> T. +31 (0)85 111 9170
> info@amarulasolutions.com
>
> www.amarulasolutions.com
>
>
>
> On Mon, Dec 18, 2023 at 10:22 AM Yann E. MORIN <yann.morin.1998@free.fr>
> wrote:
>
> > Adam, Peter, All,
> >
> > On 2023-11-02 12:41 -0600, Adam Duskett spake thusly:
> > > From: Peter Seiderer <ps.report@gmx.net>
> >
> > So, this patch is "From Peter", but...
> >
> > > Systemd creates a /etc/locale.conf file with LANG="C.UTF-8". On boot,
> > systemd
> > > reads /etc/locale.conf and sets the LANG environment variable,
> > > (see the locale_context_load_conf method in local-setup.c.)
> > >
> > > When initdb.c is called, a check for the LANG environment variable is
> > called,
> > > and if it is set to something other than "C" initdb attempts to load the
> > > corresponding LC_CTYPE file in /usr/lib/locale/. IE: If LANG is set to
> > C.UTF-8,
> > > then initdb.c attempts to load /usr/lib/locale/C.UTF-8/LC_CTYPE.
> > However, these
> > > files do not exist on a Buildroot system, and as such, initdb throws the
> > > following error on startup:
> > >
> > > ```
> > > initdb: error: invalid locale settings; check LANG and LC_* environment
> > variables
> > > pg_ctl: database system initialization failed
> > > ```
> > >
> > > To fix this issue, add "Environment=LANG=C" to the package provided
> > > postgresql.service file to force Postgresql to use the C locale.
> > >
> > > Tested-by: Adam Duskett <adam.duskett@amarulasolutions.com>
> > > Signed-off-by: Adam Duskett <adam.duskett@amarulasolutions.com>
> >
> > ... the first SoB is by Adam, and there is no SoB be Peter.
> >
> > In this case, I think I understand that Adam took Peter's patch, and
> > totally rewrote it with an alternate solution. In such a case, the
> > customs is to change the authorship to the new author, and keep a
> > Co-developed-by tag to keep credits to the first author.
> >
> > So, if you can just reply to this mail stating I got things right, I can
> > fix authorship and tags when applying. If I got things wrong, then
> > please resubmit with proper tags and/or authorship.
> >
> > Also, it does not make much sense that the submitter of a patch adds
> > their own Tested-By tag, as it is actually expected that the submitter
> > did test what they sent...
> >
> > Thanks! :-)
> >
> > Regards,
> > Yann E. MORIN.
> >
> > > ---
> > > v1 -> v2:
> > > - Get to the root cause of the problem and provide a better
> > explination of
> > > what is happening.
> > >
> > > - Use Environment=LANG=C isntead of -o --locale=C
> > >
> > > package/postgresql/postgresql.service | 4 ++++
> > > 1 file changed, 4 insertions(+)
> > >
> > > diff --git a/package/postgresql/postgresql.service
> > b/package/postgresql/postgresql.service
> > > index 539eea8964..c470c7181e 100644
> > > --- a/package/postgresql/postgresql.service
> > > +++ b/package/postgresql/postgresql.service
> > > @@ -16,6 +16,10 @@ StandardOutput=syslog
> > > StandardError=syslog
> > > SyslogIdentifier=postgres
> > >
> > > +# Overwrite the LANG variable to prevent systemd from passing the LANG
> > > +# environment variable set in /etc/locale.conf.
> > > +Environment=LANG=C
> > > +
> > > ExecStartPre=/bin/sh -c "if [ ! -f /var/lib/pgsql/PG_VERSION ]; then
> > /usr/bin/pg_ctl initdb -D /var/lib/pgsql; fi"
> > > ExecStart=/usr/bin/postgres -D /var/lib/pgsql
> > > ExecReload=/usr/bin/kill -HUP $MAINPID
> > > --
> > > 2.41.0
> > >
> > > _______________________________________________
> > > buildroot mailing list
> > > buildroot@buildroot.org
> > > https://lists.buildroot.org/mailman/listinfo/buildroot
> >
> > --
> >
> > .-----------------.--------------------.------------------.--------------------.
> > | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics'
> > conspiracy: |
> > | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___
> > |
> > | +33 561 099 427 `------------.-------: X AGAINST | \e/ There is
> > no |
> > | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v
> > conspiracy. |
> >
> > '------------------------------^-------^------------------^--------------------'
> >
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot
--
.-----------------.--------------------.------------------.--------------------.
| Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ |
| +33 561 099 427 `------------.-------: X AGAINST | \e/ There is no |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Buildroot] [PATCH v2 2/2] support/testing/tests/package/test_postgresql.py: new test
2023-12-18 17:31 ` Yann E. MORIN
@ 2023-12-18 17:41 ` Adam Duskett
2023-12-18 18:10 ` Adam Duskett
2023-12-18 20:44 ` Yann E. MORIN
0 siblings, 2 replies; 14+ messages in thread
From: Adam Duskett @ 2023-12-18 17:41 UTC (permalink / raw)
To: Yann E. MORIN; +Cc: Adam Duskett, buildroot
Yann;
Thank you for reviewing the test.
On Mon, Dec 18, 2023 at 10:31 AM Yann E. MORIN <yann.morin.1998@free.fr> wrote:
>
> Adam, All,
>
> On 2023-11-02 12:41 -0600, Adam Duskett spake thusly:
> > Perform a basic check that performs the following:
> > - Check if /var/lib/pgsql/postmaster.pid exists.
> > - Check if 'psql -c SHOW server_version;' returns sucessfully.
>
> Thanks for this new runtime test!
>
> > Note: systemd takes quite a while to start up, so check the output of
> > `systemctl is-active postgresql` until it shows "active" with a timeout
> > of 15 seconds.
>
> I think that we already concluded that testing if a unit was active or
> not was not representing whether the service was actually running. In
> the fultter test case for example, the unit was active, but the flutter
> app was just keeping crashing again and again, so the test wsa failing
> to detect failure...
>
test_flutter.py:
cmd = "systemctl is-active flutter-gallery"
I am following what is in test_flutter.py...
>
> So, I don't think usingt is-active is a good way to test whether a
> service is actually running.
>
> Furthermore, the unit may be active, but the service might not yet be
> ready to serve, so again I don;t think it is still valid to reply on it.
>
> > Signed-off-by: Adam Duskett <adam.duskett@amarulasolutions.com>
> > ---
> > - Drop BR2_PER_PACKAGE_DIRECTORIES (Thomas)
>
We need a consensus on this. This shouldn't happen. Either it is OK to
enable PPD in tests or it isn't.
> Actually, I disagree with Thomas here: we added support for PPD in the
> infra, so that if PPD is enabled in a test, then TLPB is done to speed
> up the test.
>
> [--SNIP--]
> > diff --git a/support/testing/tests/package/test_postgresql.py b/support/testing/tests/package/test_postgresql.py
> > new file mode 100644
> > index 0000000000..fa692bab7b
> > --- /dev/null
> > +++ b/support/testing/tests/package/test_postgresql.py
> > @@ -0,0 +1,73 @@
> > +import os
> > +import time
> > +import infra.basetest
> > +
> > +
> > +class TestPostgreSQLInitSysV(infra.basetest.BRTest):
> > + config = infra.basetest.BASIC_TOOLCHAIN_CONFIG + \
> > + """
> > + BR2_PACKAGE_POSTGRESQL=y
> > + BR2_TARGET_ROOTFS_EXT2=y
> > + BR2_TARGET_ROOTFS_EXT2_4=y
> > + BR2_TARGET_ROOTFS_EXT2_SIZE="128M"
> > + # BR2_TARGET_ROOTFS_TAR is not set
> > + """
> > +
> > + def test_run(self):
> > + img = os.path.join(self.builddir, "images", "rootfs.ext2")
> > + self.emulator.boot(
> > + arch="armv5",
> > + kernel="builtin",
> > + options=["-drive", f"file={img},if=scsi,format=raw"],
> > + kernel_cmdline=["root=/dev/sda"])
> > + self.emulator.login()
> > +
> > + # Check if the Daemon is running
> > + self.assertRunOk("ls /var/lib/pgsql/postmaster.pid")
> > + # Check if we can connect to the database.
> > + self.assertRunOk("su postgres -c \"psql -c 'SHOW server_version;'\"")
>
> I don't understand why we need to test for the PID file before we
> connect to the daemon. If the daemon is not running, we won't be able to
> connect to it, so the test will fail, so the PID file test is redundant,
> no?
I am being thorough. I can remove the check for the PID file if you want.
>
> Regards,
> Yann E. MORIN.
>
> > +
> > +class TestPostgreSQLInitSystemd(infra.basetest.BRTest):
> > + # Taken from test_systemd.py
> > + config = """
> > + BR2_arm=y
> > + BR2_cortex_a9=y
> > + BR2_ARM_ENABLE_VFP=y
> > + BR2_TOOLCHAIN_EXTERNAL=y
> > + BR2_TOOLCHAIN_EXTERNAL_BOOTLIN=y
> > + BR2_INIT_SYSTEMD=y
> > + BR2_TARGET_GENERIC_GETTY_PORT="ttyAMA0"
> > + BR2_PACKAGE_POSTGRESQL=y
> > + BR2_TARGET_ROOTFS_EXT2=y
> > + BR2_TARGET_ROOTFS_EXT2_4=y
> > + BR2_TARGET_ROOTFS_EXT2_SIZE="128M"
> > + # BR2_TARGET_ROOTFS_TAR is not set
> > + """
> > +
> > + def test_run(self):
> > + img = os.path.join(self.builddir, "images", "rootfs.ext2")
> > + self.emulator.boot(
> > + arch="armv7",
> > + kernel="builtin",
> > + options=["-drive", f"file={img},if=sd,format=raw"],
> > + kernel_cmdline=["root=/dev/mmcblk0"])
> > + self.emulator.login()
> > +
> > + # It may take some time for PostgreSQL to finish startup. Give it at least 15 seconds.
> > + is_active = False
> > + for i in range(15):
> > + output, _ = self.emulator.run("systemctl is-active postgresql")
> > + if output[0] == "active":
> > + is_active = True
> > + break
> > + time.sleep(1)
> > + if not is_active:
> > + self.fail("postgresql failed to activate!")
> > +
> > + # Check if the Daemon is running
> > + self.assertRunOk("ls /var/lib/pgsql/postmaster.pid")
> > +
> > + # Check if we can connect to the database.
> > + self.assertRunOk("cd / && su postgres -c \"psql -c 'SHOW server_version;'\"")
> > --
> > 2.41.0
> >
> > _______________________________________________
> > buildroot mailing list
> > buildroot@buildroot.org
> > https://lists.buildroot.org/mailman/listinfo/buildroot
>
> --
> .-----------------.--------------------.------------------.--------------------.
> | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
> | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ |
> | +33 561 099 427 `------------.-------: X AGAINST | \e/ There is no |
> | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |
> '------------------------------^-------^------------------^--------------------'
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Buildroot] [PATCH v2 1/2] package/postgresql/postgresql.service: set locale for initdb to C
2023-12-18 17:39 ` Yann E. MORIN
@ 2023-12-18 17:44 ` Adam Duskett
2023-12-18 22:09 ` Adam Duskett
0 siblings, 1 reply; 14+ messages in thread
From: Adam Duskett @ 2023-12-18 17:44 UTC (permalink / raw)
To: Yann E. MORIN; +Cc: Peter Seiderer, buildroot
Adam Duskett
Senior Embedded Systems Developer
M. +1208-515-8102
adam.duskett@amarulasolutions.com
__________________________________
Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9170
info@amarulasolutions.com
www.amarulasolutions.com
Adam Duskett
Senior Embedded Systems Developer
M. +1208-515-8102
adam.duskett@amarulasolutions.com
__________________________________
Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9170
info@amarulasolutions.com
www.amarulasolutions.com
On Mon, Dec 18, 2023 at 10:39 AM Yann E. MORIN <yann.morin.1998@free.fr> wrote:
>
> Adam, All,
>
> [Please, do not top-post]
>
OK
> On 2023-12-18 10:28 -0700, Adam Duskett spake thusly:
> > I didn't add Peter as the patch is completely different than what he sent
> > along with a more
> > comprehensive explanation as to what is happening.
>
> Then you should also have dropped him as the author; note how the first
> line of the patch was still:
>
> From: Peter Seiderer <ps.report@gmx.net>
>
> ... which git uses as the author of the patch.
>
> > If you want to add Peter
> > as a
> > co-author that is more than fine by me.
>
> It is not about what I want, it is about keeping auhtorship and SoB in
> sync and coherent with who actually did what.
>
> So if I understand correctly: you are the author, so you have your SoB,
> and Peter is a co-dev for credits, right: Yes/No ? ;-)
Yes
>
> If yes, then can I fix the authorship and add the Co-developed-by tag
> for Peter: Yes/No?
>
> If no to either, can you respin a fixed-up patch, please?
>
Adam
> Regards,
> Yann E. MORIN.
>
> > Adam Duskett
> >
> > Senior Embedded Systems Developer
> >
> > M. +1208-515-8102
> >
> > adam.duskett@amarulasolutions.com
> >
> > __________________________________
> >
> >
> > Amarula Solutions BV
> >
> > Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
> >
> > T. +31 (0)85 111 9170
> > info@amarulasolutions.com
> >
> > www.amarulasolutions.com
> >
> >
> >
> > On Mon, Dec 18, 2023 at 10:22 AM Yann E. MORIN <yann.morin.1998@free.fr>
> > wrote:
> >
> > > Adam, Peter, All,
> > >
> > > On 2023-11-02 12:41 -0600, Adam Duskett spake thusly:
> > > > From: Peter Seiderer <ps.report@gmx.net>
> > >
> > > So, this patch is "From Peter", but...
> > >
> > > > Systemd creates a /etc/locale.conf file with LANG="C.UTF-8". On boot,
> > > systemd
> > > > reads /etc/locale.conf and sets the LANG environment variable,
> > > > (see the locale_context_load_conf method in local-setup.c.)
> > > >
> > > > When initdb.c is called, a check for the LANG environment variable is
> > > called,
> > > > and if it is set to something other than "C" initdb attempts to load the
> > > > corresponding LC_CTYPE file in /usr/lib/locale/. IE: If LANG is set to
> > > C.UTF-8,
> > > > then initdb.c attempts to load /usr/lib/locale/C.UTF-8/LC_CTYPE.
> > > However, these
> > > > files do not exist on a Buildroot system, and as such, initdb throws the
> > > > following error on startup:
> > > >
> > > > ```
> > > > initdb: error: invalid locale settings; check LANG and LC_* environment
> > > variables
> > > > pg_ctl: database system initialization failed
> > > > ```
> > > >
> > > > To fix this issue, add "Environment=LANG=C" to the package provided
> > > > postgresql.service file to force Postgresql to use the C locale.
> > > >
> > > > Tested-by: Adam Duskett <adam.duskett@amarulasolutions.com>
> > > > Signed-off-by: Adam Duskett <adam.duskett@amarulasolutions.com>
> > >
> > > ... the first SoB is by Adam, and there is no SoB be Peter.
> > >
> > > In this case, I think I understand that Adam took Peter's patch, and
> > > totally rewrote it with an alternate solution. In such a case, the
> > > customs is to change the authorship to the new author, and keep a
> > > Co-developed-by tag to keep credits to the first author.
> > >
> > > So, if you can just reply to this mail stating I got things right, I can
> > > fix authorship and tags when applying. If I got things wrong, then
> > > please resubmit with proper tags and/or authorship.
> > >
> > > Also, it does not make much sense that the submitter of a patch adds
> > > their own Tested-By tag, as it is actually expected that the submitter
> > > did test what they sent...
> > >
> > > Thanks! :-)
> > >
> > > Regards,
> > > Yann E. MORIN.
> > >
> > > > ---
> > > > v1 -> v2:
> > > > - Get to the root cause of the problem and provide a better
> > > explination of
> > > > what is happening.
> > > >
> > > > - Use Environment=LANG=C isntead of -o --locale=C
> > > >
> > > > package/postgresql/postgresql.service | 4 ++++
> > > > 1 file changed, 4 insertions(+)
> > > >
> > > > diff --git a/package/postgresql/postgresql.service
> > > b/package/postgresql/postgresql.service
> > > > index 539eea8964..c470c7181e 100644
> > > > --- a/package/postgresql/postgresql.service
> > > > +++ b/package/postgresql/postgresql.service
> > > > @@ -16,6 +16,10 @@ StandardOutput=syslog
> > > > StandardError=syslog
> > > > SyslogIdentifier=postgres
> > > >
> > > > +# Overwrite the LANG variable to prevent systemd from passing the LANG
> > > > +# environment variable set in /etc/locale.conf.
> > > > +Environment=LANG=C
> > > > +
> > > > ExecStartPre=/bin/sh -c "if [ ! -f /var/lib/pgsql/PG_VERSION ]; then
> > > /usr/bin/pg_ctl initdb -D /var/lib/pgsql; fi"
> > > > ExecStart=/usr/bin/postgres -D /var/lib/pgsql
> > > > ExecReload=/usr/bin/kill -HUP $MAINPID
> > > > --
> > > > 2.41.0
> > > >
> > > > _______________________________________________
> > > > buildroot mailing list
> > > > buildroot@buildroot.org
> > > > https://lists.buildroot.org/mailman/listinfo/buildroot
> > >
> > > --
> > >
> > > .-----------------.--------------------.------------------.--------------------.
> > > | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics'
> > > conspiracy: |
> > > | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___
> > > |
> > > | +33 561 099 427 `------------.-------: X AGAINST | \e/ There is
> > > no |
> > > | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v
> > > conspiracy. |
> > >
> > > '------------------------------^-------^------------------^--------------------'
> > >
>
> > _______________________________________________
> > buildroot mailing list
> > buildroot@buildroot.org
> > https://lists.buildroot.org/mailman/listinfo/buildroot
>
>
> --
> .-----------------.--------------------.------------------.--------------------.
> | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
> | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ |
> | +33 561 099 427 `------------.-------: X AGAINST | \e/ There is no |
> | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |
> '------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Buildroot] [PATCH v2 2/2] support/testing/tests/package/test_postgresql.py: new test
2023-12-18 17:41 ` Adam Duskett
@ 2023-12-18 18:10 ` Adam Duskett
2023-12-18 20:44 ` Yann E. MORIN
1 sibling, 0 replies; 14+ messages in thread
From: Adam Duskett @ 2023-12-18 18:10 UTC (permalink / raw)
To: Adam Duskett; +Cc: Yann E. MORIN, buildroot
On Mon, Dec 18, 2023 at 10:41 AM Adam Duskett <aduskett@gmail.com> wrote:
>
> Yann;
>
> Thank you for reviewing the test.
>
> On Mon, Dec 18, 2023 at 10:31 AM Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> >
> > Adam, All,
> >
> > On 2023-11-02 12:41 -0600, Adam Duskett spake thusly:
> > > Perform a basic check that performs the following:
> > > - Check if /var/lib/pgsql/postmaster.pid exists.
> > > - Check if 'psql -c SHOW server_version;' returns sucessfully.
> >
> > Thanks for this new runtime test!
> >
> > > Note: systemd takes quite a while to start up, so check the output of
> > > `systemctl is-active postgresql` until it shows "active" with a timeout
> > > of 15 seconds.
> >
> > I think that we already concluded that testing if a unit was active or
> > not was not representing whether the service was actually running. In
> > the fultter test case for example, the unit was active, but the flutter
> > app was just keeping crashing again and again, so the test wsa failing
> > to detect failure...
> >
> test_flutter.py:
> cmd = "systemctl is-active flutter-gallery"
> I am following what is in test_flutter.py...
To add to this, running a query on pgsql would fail if pgsql isn't running,
so this is perfectly safe.
>
> >
> > So, I don't think usingt is-active is a good way to test whether a
> > service is actually running.
> >
> > Furthermore, the unit may be active, but the service might not yet be
> > ready to serve, so again I don;t think it is still valid to reply on it.
> >
> > > Signed-off-by: Adam Duskett <adam.duskett@amarulasolutions.com>
> > > ---
> > > - Drop BR2_PER_PACKAGE_DIRECTORIES (Thomas)
> >
> We need a consensus on this. This shouldn't happen. Either it is OK to
> enable PPD in tests or it isn't.
>
> > Actually, I disagree with Thomas here: we added support for PPD in the
> > infra, so that if PPD is enabled in a test, then TLPB is done to speed
> > up the test.
> >
> > [--SNIP--]
> > > diff --git a/support/testing/tests/package/test_postgresql.py b/support/testing/tests/package/test_postgresql.py
> > > new file mode 100644
> > > index 0000000000..fa692bab7b
> > > --- /dev/null
> > > +++ b/support/testing/tests/package/test_postgresql.py
> > > @@ -0,0 +1,73 @@
> > > +import os
> > > +import time
> > > +import infra.basetest
> > > +
> > > +
> > > +class TestPostgreSQLInitSysV(infra.basetest.BRTest):
> > > + config = infra.basetest.BASIC_TOOLCHAIN_CONFIG + \
> > > + """
> > > + BR2_PACKAGE_POSTGRESQL=y
> > > + BR2_TARGET_ROOTFS_EXT2=y
> > > + BR2_TARGET_ROOTFS_EXT2_4=y
> > > + BR2_TARGET_ROOTFS_EXT2_SIZE="128M"
> > > + # BR2_TARGET_ROOTFS_TAR is not set
> > > + """
> > > +
> > > + def test_run(self):
> > > + img = os.path.join(self.builddir, "images", "rootfs.ext2")
> > > + self.emulator.boot(
> > > + arch="armv5",
> > > + kernel="builtin",
> > > + options=["-drive", f"file={img},if=scsi,format=raw"],
> > > + kernel_cmdline=["root=/dev/sda"])
> > > + self.emulator.login()
> > > +
> > > + # Check if the Daemon is running
> > > + self.assertRunOk("ls /var/lib/pgsql/postmaster.pid")
> > > + # Check if we can connect to the database.
> > > + self.assertRunOk("su postgres -c \"psql -c 'SHOW server_version;'\"")
> >
> > I don't understand why we need to test for the PID file before we
> > connect to the daemon. If the daemon is not running, we won't be able to
> > connect to it, so the test will fail, so the PID file test is redundant,
> > no?
> I am being thorough. I can remove the check for the PID file if you want.
> >
> > Regards,
> > Yann E. MORIN.
> >
> > > +
> > > +class TestPostgreSQLInitSystemd(infra.basetest.BRTest):
> > > + # Taken from test_systemd.py
> > > + config = """
> > > + BR2_arm=y
> > > + BR2_cortex_a9=y
> > > + BR2_ARM_ENABLE_VFP=y
> > > + BR2_TOOLCHAIN_EXTERNAL=y
> > > + BR2_TOOLCHAIN_EXTERNAL_BOOTLIN=y
> > > + BR2_INIT_SYSTEMD=y
> > > + BR2_TARGET_GENERIC_GETTY_PORT="ttyAMA0"
> > > + BR2_PACKAGE_POSTGRESQL=y
> > > + BR2_TARGET_ROOTFS_EXT2=y
> > > + BR2_TARGET_ROOTFS_EXT2_4=y
> > > + BR2_TARGET_ROOTFS_EXT2_SIZE="128M"
> > > + # BR2_TARGET_ROOTFS_TAR is not set
> > > + """
> > > +
> > > + def test_run(self):
> > > + img = os.path.join(self.builddir, "images", "rootfs.ext2")
> > > + self.emulator.boot(
> > > + arch="armv7",
> > > + kernel="builtin",
> > > + options=["-drive", f"file={img},if=sd,format=raw"],
> > > + kernel_cmdline=["root=/dev/mmcblk0"])
> > > + self.emulator.login()
> > > +
> > > + # It may take some time for PostgreSQL to finish startup. Give it at least 15 seconds.
> > > + is_active = False
> > > + for i in range(15):
> > > + output, _ = self.emulator.run("systemctl is-active postgresql")
> > > + if output[0] == "active":
> > > + is_active = True
> > > + break
> > > + time.sleep(1)
> > > + if not is_active:
> > > + self.fail("postgresql failed to activate!")
> > > +
> > > + # Check if the Daemon is running
> > > + self.assertRunOk("ls /var/lib/pgsql/postmaster.pid")
> > > +
> > > + # Check if we can connect to the database.
> > > + self.assertRunOk("cd / && su postgres -c \"psql -c 'SHOW server_version;'\"")
> > > --
> > > 2.41.0
> > >
> > > _______________________________________________
> > > buildroot mailing list
> > > buildroot@buildroot.org
> > > https://lists.buildroot.org/mailman/listinfo/buildroot
> >
> > --
> > .-----------------.--------------------.------------------.--------------------.
> > | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
> > | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ |
> > | +33 561 099 427 `------------.-------: X AGAINST | \e/ There is no |
> > | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |
> > '------------------------------^-------^------------------^--------------------'
> > _______________________________________________
> > buildroot mailing list
> > buildroot@buildroot.org
> > https://lists.buildroot.org/mailman/listinfo/buildroot
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Buildroot] [PATCH v2 2/2] support/testing/tests/package/test_postgresql.py: new test
2023-12-18 17:41 ` Adam Duskett
2023-12-18 18:10 ` Adam Duskett
@ 2023-12-18 20:44 ` Yann E. MORIN
2023-12-18 21:40 ` Adam Duskett
1 sibling, 1 reply; 14+ messages in thread
From: Yann E. MORIN @ 2023-12-18 20:44 UTC (permalink / raw)
To: Adam Duskett; +Cc: Adam Duskett, buildroot
Adam, All,
On 2023-12-18 10:41 -0700, Adam Duskett spake thusly:
> On Mon, Dec 18, 2023 at 10:31 AM Yann E. MORIN <yann.morin.1998@free.fr> wrote:
[--SNIP--]
> > I think that we already concluded that testing if a unit was active or
> > not was not representing whether the service was actually running. In
> > the fultter test case for example, the unit was active, but the flutter
> > app was just keeping crashing again and again, so the test wsa failing
> > to detect failure...
> test_flutter.py:
> cmd = "systemctl is-active flutter-gallery"
> I am following what is in test_flutter.py...
Exactly, and as I explained on IRC (and already explained, and reported
with [0]), it does not work: the unit is active, but the application the
service runs is constantly crashing.
So no, having a unit reported as active is not an indication of success.
It is merely an indication that systemd did schedule and start that
unit, not that the service is running appropriately.
[0] https://lore.kernel.org/buildroot/7cf874f127f56c8132862163aa2c4d44f6f39efe.1696091295.git.yann.morin.1998@free.fr/
> > > - Drop BR2_PER_PACKAGE_DIRECTORIES (Thomas)
> > Actually, I disagree with Thomas here: we added support for PPD in the
> > infra, so that if PPD is enabled in a test, then TLPB is done to speed
> > up the test.
> We need a consensus on this. This shouldn't happen. Either it is OK to
> enable PPD in tests or it isn't.
I would say we usually do not want it, unless the runtime test "take too
long to run" and thus we enable PPD. But it has to be justified (even
briefly) in the comit log (e.g. "enabled PPD to divide the build time by
N"; N=3 seems the minimum to justify PPD, I'd say)
[--SNIP--]
> > > + self.assertRunOk("su postgres -c \"psql -c 'SHOW server_version;'\"")
> > I don't understand why we need to test for the PID file before we
> > connect to the daemon. If the daemon is not running, we won't be able to
> > connect to it, so the test will fail, so the PID file test is redundant,
> > no?
> I am being thorough. I can remove the check for the PID file if you want.
I see. However, what I wonder is: what does it bring, from the
integration in Buildroot perspective? What I mean is:
- a runtime test is meant to validate that the integration in
Buildroot is delivering a package that is working, and working as
expected, at least in its basic features;
- a runtime test shall fail when the expectations are not met, with a
proper report of what is broken.
So, in this case, querying the DB should be enough to check that the
daemon is running: if it is not running, then the query will fail with
the appropriate message, which is going to be different from the case
where the DB does not exist, or where the access to the DB has been
denied by lack of proper credentials, or...
If a failure to query the DB can't indeed differentiate the daemon-is-
not-running case from the others, then indeed we need another way. But
then we need explanations about that, however brief they be.
Regards,
Yann E. MORIN.
--
.-----------------.--------------------.------------------.--------------------.
| Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ |
| +33 561 099 427 `------------.-------: X AGAINST | \e/ There is no |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Buildroot] [PATCH v2 2/2] support/testing/tests/package/test_postgresql.py: new test
2023-12-18 20:44 ` Yann E. MORIN
@ 2023-12-18 21:40 ` Adam Duskett
0 siblings, 0 replies; 14+ messages in thread
From: Adam Duskett @ 2023-12-18 21:40 UTC (permalink / raw)
To: Yann E. MORIN; +Cc: Adam Duskett, buildroot
Yann;
On Mon, Dec 18, 2023 at 1:44 PM Yann E. MORIN <yann.morin.1998@free.fr> wrote:
>
> Adam, All,
>
> On 2023-12-18 10:41 -0700, Adam Duskett spake thusly:
> > On Mon, Dec 18, 2023 at 10:31 AM Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> [--SNIP--]
> > > I think that we already concluded that testing if a unit was active or
> > > not was not representing whether the service was actually running. In
> > > the fultter test case for example, the unit was active, but the flutter
> > > app was just keeping crashing again and again, so the test wsa failing
> > > to detect failure...
> > test_flutter.py:
> > cmd = "systemctl is-active flutter-gallery"
> > I am following what is in test_flutter.py...
>
> Exactly, and as I explained on IRC (and already explained, and reported
> with [0]), it does not work: the unit is active, but the application the
> service runs is constantly crashing.
>
> So no, having a unit reported as active is not an indication of success.
> It is merely an indication that systemd did schedule and start that
> unit, not that the service is running appropriately.
>
> [0] https://lore.kernel.org/buildroot/7cf874f127f56c8132862163aa2c4d44f6f39efe.1696091295.git.yann.morin.1998@free.fr/
>
> > > > - Drop BR2_PER_PACKAGE_DIRECTORIES (Thomas)
> > > Actually, I disagree with Thomas here: we added support for PPD in the
> > > infra, so that if PPD is enabled in a test, then TLPB is done to speed
> > > up the test.
> > We need a consensus on this. This shouldn't happen. Either it is OK to
> > enable PPD in tests or it isn't.
>
> I would say we usually do not want it, unless the runtime test "take too
> long to run" and thus we enable PPD. But it has to be justified (even
> briefly) in the comit log (e.g. "enabled PPD to divide the build time by
> N"; N=3 seems the minimum to justify PPD, I'd say)
>
> [--SNIP--]
> > > > + self.assertRunOk("su postgres -c \"psql -c 'SHOW server_version;'\"")
> > > I don't understand why we need to test for the PID file before we
> > > connect to the daemon. If the daemon is not running, we won't be able to
> > > connect to it, so the test will fail, so the PID file test is redundant,
> > > no?
> > I am being thorough. I can remove the check for the PID file if you want.
>
> I see. However, what I wonder is: what does it bring, from the
> integration in Buildroot perspective? What I mean is:
>
> - a runtime test is meant to validate that the integration in
> Buildroot is delivering a package that is working, and working as
> expected, at least in its basic features;
>
> - a runtime test shall fail when the expectations are not met, with a
> proper report of what is broken.
>
> So, in this case, querying the DB should be enough to check that the
> daemon is running: if it is not running, then the query will fail with
> the appropriate message, which is going to be different from the case
> where the DB does not exist, or where the access to the DB has been
> denied by lack of proper credentials, or...
>
> If a failure to query the DB can't indeed differentiate the daemon-is-
> not-running case from the others, then indeed we need another way. But
> then we need explanations about that, however brief they be.
>
I'm going to be real with you. I don't even use pgsql in my projects.
I was trying
to help because I noticed that it was broken, but this is far too much
bikeshedding
and nitpicking and I don't have the desire nor time to redo the test.
I'll decline the
patch and if someone else wants to make a test they are free to do so.
Adam
> Regards,
> Yann E. MORIN.
>
> --
> .-----------------.--------------------.------------------.--------------------.
> | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
> | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ |
> | +33 561 099 427 `------------.-------: X AGAINST | \e/ There is no |
> | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |
> '------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Buildroot] [PATCH v2 1/2] package/postgresql/postgresql.service: set locale for initdb to C
2023-12-18 17:44 ` Adam Duskett
@ 2023-12-18 22:09 ` Adam Duskett
2023-12-24 22:10 ` Adam Duskett
0 siblings, 1 reply; 14+ messages in thread
From: Adam Duskett @ 2023-12-18 22:09 UTC (permalink / raw)
To: Yann E. MORIN; +Cc: Peter Seiderer, buildroot
Sorry, Gmail defaulted to HTML.
> On Mon, Dec 18, 2023 at 10:39 AM Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> >
> > Adam, All,
> >
> > [Please, do not top-post]
> >
OK
> > On 2023-12-18 10:28 -0700, Adam Duskett spake thusly:
> > > I didn't add Peter as the patch is completely different than what he sent
> > > along with a more
> > > comprehensive explanation as to what is happening.
> >
> > Then you should also have dropped him as the author; note how the first
> > line of the patch was still:
> >
> > From: Peter Seiderer <ps.report@gmx.net>
> >
> > ... which git uses as the author of the patch.
> >
> > > If you want to add Peter
> > > as a
> > > co-author that is more than fine by me.
> >
> > It is not about what I want, it is about keeping auhtorship and SoB in
> > sync and coherent with who actually did what.
> >
> > So if I understand correctly: you are the author, so you have your SoB,
> > and Peter is a co-dev for credits, right: Yes/No ? ;-)
Yes
> >
> > If yes, then can I fix the authorship and add the Co-developed-by tag
> > for Peter: Yes/No?
> >
> > If no to either, can you respin a fixed-up patch, please?
> >
> Adam
> > Regards,
> > Yann E. MORIN.
> >
> > > Adam Duskett
> > >
> > > Senior Embedded Systems Developer
> > >
> > > M. +1208-515-8102
> > >
> > > adam.duskett@amarulasolutions.com
> > >
> > > __________________________________
> > >
> > >
> > > Amarula Solutions BV
> > >
> > > Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
> > >
> > > T. +31 (0)85 111 9170
> > > info@amarulasolutions.com
> > >
> > > www.amarulasolutions.com
> > >
> > >
> > >
> > > On Mon, Dec 18, 2023 at 10:22 AM Yann E. MORIN <yann.morin.1998@free.fr>
> > > wrote:
> > >
> > > > Adam, Peter, All,
> > > >
> > > > On 2023-11-02 12:41 -0600, Adam Duskett spake thusly:
> > > > > From: Peter Seiderer <ps.report@gmx.net>
> > > >
> > > > So, this patch is "From Peter", but...
> > > >
> > > > > Systemd creates a /etc/locale.conf file with LANG="C.UTF-8". On boot,
> > > > systemd
> > > > > reads /etc/locale.conf and sets the LANG environment variable,
> > > > > (see the locale_context_load_conf method in local-setup.c.)
> > > > >
> > > > > When initdb.c is called, a check for the LANG environment variable is
> > > > called,
> > > > > and if it is set to something other than "C" initdb attempts to load the
> > > > > corresponding LC_CTYPE file in /usr/lib/locale/. IE: If LANG is set to
> > > > C.UTF-8,
> > > > > then initdb.c attempts to load /usr/lib/locale/C.UTF-8/LC_CTYPE.
> > > > However, these
> > > > > files do not exist on a Buildroot system, and as such, initdb throws the
> > > > > following error on startup:
> > > > >
> > > > > ```
> > > > > initdb: error: invalid locale settings; check LANG and LC_* environment
> > > > variables
> > > > > pg_ctl: database system initialization failed
> > > > > ```
> > > > >
> > > > > To fix this issue, add "Environment=LANG=C" to the package provided
> > > > > postgresql.service file to force Postgresql to use the C locale.
> > > > >
> > > > > Tested-by: Adam Duskett <adam.duskett@amarulasolutions.com>
> > > > > Signed-off-by: Adam Duskett <adam.duskett@amarulasolutions.com>
> > > >
> > > > ... the first SoB is by Adam, and there is no SoB be Peter.
> > > >
> > > > In this case, I think I understand that Adam took Peter's patch, and
> > > > totally rewrote it with an alternate solution. In such a case, the
> > > > customs is to change the authorship to the new author, and keep a
> > > > Co-developed-by tag to keep credits to the first author.
> > > >
> > > > So, if you can just reply to this mail stating I got things right, I can
> > > > fix authorship and tags when applying. If I got things wrong, then
> > > > please resubmit with proper tags and/or authorship.
> > > >
> > > > Also, it does not make much sense that the submitter of a patch adds
> > > > their own Tested-By tag, as it is actually expected that the submitter
> > > > did test what they sent...
> > > >
> > > > Thanks! :-)
> > > >
> > > > Regards,
> > > > Yann E. MORIN.
> > > >
> > > > > ---
> > > > > v1 -> v2:
> > > > > - Get to the root cause of the problem and provide a better
> > > > explination of
> > > > > what is happening.
> > > > >
> > > > > - Use Environment=LANG=C isntead of -o --locale=C
> > > > >
> > > > > package/postgresql/postgresql.service | 4 ++++
> > > > > 1 file changed, 4 insertions(+)
> > > > >
> > > > > diff --git a/package/postgresql/postgresql.service
> > > > b/package/postgresql/postgresql.service
> > > > > index 539eea8964..c470c7181e 100644
> > > > > --- a/package/postgresql/postgresql.service
> > > > > +++ b/package/postgresql/postgresql.service
> > > > > @@ -16,6 +16,10 @@ StandardOutput=syslog
> > > > > StandardError=syslog
> > > > > SyslogIdentifier=postgres
> > > > >
> > > > > +# Overwrite the LANG variable to prevent systemd from passing the LANG
> > > > > +# environment variable set in /etc/locale.conf.
> > > > > +Environment=LANG=C
> > > > > +
> > > > > ExecStartPre=/bin/sh -c "if [ ! -f /var/lib/pgsql/PG_VERSION ]; then
> > > > /usr/bin/pg_ctl initdb -D /var/lib/pgsql; fi"
> > > > > ExecStart=/usr/bin/postgres -D /var/lib/pgsql
> > > > > ExecReload=/usr/bin/kill -HUP $MAINPID
> > > > > --
> > > > > 2.41.0
> > > > >
> > > > > _______________________________________________
> > > > > buildroot mailing list
> > > > > buildroot@buildroot.org
> > > > > https://lists.buildroot.org/mailman/listinfo/buildroot
> > > >
> > > > --
> > > >
> > > > .-----------------.--------------------.------------------.--------------------.
> > > > | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics'
> > > > conspiracy: |
> > > > | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___
> > > > |
> > > > | +33 561 099 427 `------------.-------: X AGAINST | \e/ There is
> > > > no |
> > > > | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v
> > > > conspiracy. |
> > > >
> > > > '------------------------------^-------^------------------^--------------------'
> > > >
> >
> > > _______________________________________________
> > > buildroot mailing list
> > > buildroot@buildroot.org
> > > https://lists.buildroot.org/mailman/listinfo/buildroot
> >
> >
> > --
> > .-----------------.--------------------.------------------.--------------------.
> > | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
> > | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ |
> > | +33 561 099 427 `------------.-------: X AGAINST | \e/ There is no |
> > | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |
> > '------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Buildroot] [PATCH v2 1/2] package/postgresql/postgresql.service: set locale for initdb to C
2023-12-18 22:09 ` Adam Duskett
@ 2023-12-24 22:10 ` Adam Duskett
0 siblings, 0 replies; 14+ messages in thread
From: Adam Duskett @ 2023-12-24 22:10 UTC (permalink / raw)
To: Adam Duskett; +Cc: Peter Seiderer, Yann E. MORIN, buildroot
I will mark this series as rejected, as it's not Flutter-related and
Amarula does not currently use PGSQL.
On Mon, Dec 18, 2023 at 3:10 PM Adam Duskett
<adam.duskett@amarulasolutions.com> wrote:
>
> Sorry, Gmail defaulted to HTML.
>
> > On Mon, Dec 18, 2023 at 10:39 AM Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> > >
> > > Adam, All,
> > >
> > > [Please, do not top-post]
> > >
> OK
> > > On 2023-12-18 10:28 -0700, Adam Duskett spake thusly:
> > > > I didn't add Peter as the patch is completely different than what he sent
> > > > along with a more
> > > > comprehensive explanation as to what is happening.
> > >
> > > Then you should also have dropped him as the author; note how the first
> > > line of the patch was still:
> > >
> > > From: Peter Seiderer <ps.report@gmx.net>
> > >
> > > ... which git uses as the author of the patch.
> > >
> > > > If you want to add Peter
> > > > as a
> > > > co-author that is more than fine by me.
> > >
> > > It is not about what I want, it is about keeping auhtorship and SoB in
> > > sync and coherent with who actually did what.
> > >
> > > So if I understand correctly: you are the author, so you have your SoB,
> > > and Peter is a co-dev for credits, right: Yes/No ? ;-)
> Yes
> > >
> > > If yes, then can I fix the authorship and add the Co-developed-by tag
> > > for Peter: Yes/No?
> > >
> > > If no to either, can you respin a fixed-up patch, please?
> > >
> > Adam
> > > Regards,
> > > Yann E. MORIN.
> > >
> > > > Adam Duskett
> > > >
> > > > Senior Embedded Systems Developer
> > > >
> > > > M. +1208-515-8102
> > > >
> > > > adam.duskett@amarulasolutions.com
> > > >
> > > > __________________________________
> > > >
> > > >
> > > > Amarula Solutions BV
> > > >
> > > > Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
> > > >
> > > > T. +31 (0)85 111 9170
> > > > info@amarulasolutions.com
> > > >
> > > > www.amarulasolutions.com
> > > >
> > > >
> > > >
> > > > On Mon, Dec 18, 2023 at 10:22 AM Yann E. MORIN <yann.morin.1998@free.fr>
> > > > wrote:
> > > >
> > > > > Adam, Peter, All,
> > > > >
> > > > > On 2023-11-02 12:41 -0600, Adam Duskett spake thusly:
> > > > > > From: Peter Seiderer <ps.report@gmx.net>
> > > > >
> > > > > So, this patch is "From Peter", but...
> > > > >
> > > > > > Systemd creates a /etc/locale.conf file with LANG="C.UTF-8". On boot,
> > > > > systemd
> > > > > > reads /etc/locale.conf and sets the LANG environment variable,
> > > > > > (see the locale_context_load_conf method in local-setup.c.)
> > > > > >
> > > > > > When initdb.c is called, a check for the LANG environment variable is
> > > > > called,
> > > > > > and if it is set to something other than "C" initdb attempts to load the
> > > > > > corresponding LC_CTYPE file in /usr/lib/locale/. IE: If LANG is set to
> > > > > C.UTF-8,
> > > > > > then initdb.c attempts to load /usr/lib/locale/C.UTF-8/LC_CTYPE.
> > > > > However, these
> > > > > > files do not exist on a Buildroot system, and as such, initdb throws the
> > > > > > following error on startup:
> > > > > >
> > > > > > ```
> > > > > > initdb: error: invalid locale settings; check LANG and LC_* environment
> > > > > variables
> > > > > > pg_ctl: database system initialization failed
> > > > > > ```
> > > > > >
> > > > > > To fix this issue, add "Environment=LANG=C" to the package provided
> > > > > > postgresql.service file to force Postgresql to use the C locale.
> > > > > >
> > > > > > Tested-by: Adam Duskett <adam.duskett@amarulasolutions.com>
> > > > > > Signed-off-by: Adam Duskett <adam.duskett@amarulasolutions.com>
> > > > >
> > > > > ... the first SoB is by Adam, and there is no SoB be Peter.
> > > > >
> > > > > In this case, I think I understand that Adam took Peter's patch, and
> > > > > totally rewrote it with an alternate solution. In such a case, the
> > > > > customs is to change the authorship to the new author, and keep a
> > > > > Co-developed-by tag to keep credits to the first author.
> > > > >
> > > > > So, if you can just reply to this mail stating I got things right, I can
> > > > > fix authorship and tags when applying. If I got things wrong, then
> > > > > please resubmit with proper tags and/or authorship.
> > > > >
> > > > > Also, it does not make much sense that the submitter of a patch adds
> > > > > their own Tested-By tag, as it is actually expected that the submitter
> > > > > did test what they sent...
> > > > >
> > > > > Thanks! :-)
> > > > >
> > > > > Regards,
> > > > > Yann E. MORIN.
> > > > >
> > > > > > ---
> > > > > > v1 -> v2:
> > > > > > - Get to the root cause of the problem and provide a better
> > > > > explination of
> > > > > > what is happening.
> > > > > >
> > > > > > - Use Environment=LANG=C isntead of -o --locale=C
> > > > > >
> > > > > > package/postgresql/postgresql.service | 4 ++++
> > > > > > 1 file changed, 4 insertions(+)
> > > > > >
> > > > > > diff --git a/package/postgresql/postgresql.service
> > > > > b/package/postgresql/postgresql.service
> > > > > > index 539eea8964..c470c7181e 100644
> > > > > > --- a/package/postgresql/postgresql.service
> > > > > > +++ b/package/postgresql/postgresql.service
> > > > > > @@ -16,6 +16,10 @@ StandardOutput=syslog
> > > > > > StandardError=syslog
> > > > > > SyslogIdentifier=postgres
> > > > > >
> > > > > > +# Overwrite the LANG variable to prevent systemd from passing the LANG
> > > > > > +# environment variable set in /etc/locale.conf.
> > > > > > +Environment=LANG=C
> > > > > > +
> > > > > > ExecStartPre=/bin/sh -c "if [ ! -f /var/lib/pgsql/PG_VERSION ]; then
> > > > > /usr/bin/pg_ctl initdb -D /var/lib/pgsql; fi"
> > > > > > ExecStart=/usr/bin/postgres -D /var/lib/pgsql
> > > > > > ExecReload=/usr/bin/kill -HUP $MAINPID
> > > > > > --
> > > > > > 2.41.0
> > > > > >
> > > > > > _______________________________________________
> > > > > > buildroot mailing list
> > > > > > buildroot@buildroot.org
> > > > > > https://lists.buildroot.org/mailman/listinfo/buildroot
> > > > >
> > > > > --
> > > > >
> > > > > .-----------------.--------------------.------------------.--------------------.
> > > > > | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics'
> > > > > conspiracy: |
> > > > > | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___
> > > > > |
> > > > > | +33 561 099 427 `------------.-------: X AGAINST | \e/ There is
> > > > > no |
> > > > > | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v
> > > > > conspiracy. |
> > > > >
> > > > > '------------------------------^-------^------------------^--------------------'
> > > > >
> > >
> > > > _______________________________________________
> > > > buildroot mailing list
> > > > buildroot@buildroot.org
> > > > https://lists.buildroot.org/mailman/listinfo/buildroot
> > >
> > >
> > > --
> > > .-----------------.--------------------.------------------.--------------------.
> > > | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
> > > | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ |
> > > | +33 561 099 427 `------------.-------: X AGAINST | \e/ There is no |
> > > | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |
> > > '------------------------------^-------^------------------^--------------------'
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2023-12-24 22:10 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-11-02 18:41 [Buildroot] [PATCH v2 1/2] package/postgresql/postgresql.service: set locale for initdb to C Adam Duskett
2023-11-02 18:41 ` [Buildroot] [PATCH v2 2/2] support/testing/tests/package/test_postgresql.py: new test Adam Duskett
2023-12-18 17:31 ` Yann E. MORIN
2023-12-18 17:41 ` Adam Duskett
2023-12-18 18:10 ` Adam Duskett
2023-12-18 20:44 ` Yann E. MORIN
2023-12-18 21:40 ` Adam Duskett
2023-11-05 10:07 ` [Buildroot] [PATCH v2 1/2] package/postgresql/postgresql.service: set locale for initdb to C Peter Seiderer
2023-12-18 17:22 ` Yann E. MORIN
2023-12-18 17:28 ` Adam Duskett
2023-12-18 17:39 ` Yann E. MORIN
2023-12-18 17:44 ` Adam Duskett
2023-12-18 22:09 ` Adam Duskett
2023-12-24 22:10 ` Adam Duskett
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.