Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH 1/5] support/scripts/mkusers: fix UID/GID confusion
  2024-03-09 23:44 [Buildroot] [PATCH 0/5] support: accept text files with missing terminating \n (branch yem/newlines) Yann E. MORIN
@ 2024-03-09 23:44 ` Yann E. MORIN
  2024-03-09 23:44 ` [Buildroot] [PATCH 2/5] support/scripts/mkusers: fix shellcheck errors Yann E. MORIN
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Yann E. MORIN @ 2024-03-09 23:44 UTC (permalink / raw)
  To: buildroot; +Cc: Norbert Lange, Yann E. MORIN

Commit 41ea61d59c3e (support/scripts/mkusers: allow option for system
uid/gid) confused GID and UID variables: the GID limits were used to
create UIDs.

Fix that.

Note that this fixes a shellcheck error; although there are many more
shellcheck errors, these fixes are semantically a bug that need to be
fixed separately from the coding style issues reported by shellcheck.

Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
Cc: Norbert Lange <nolange79@gmail.com>
Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
 support/scripts/mkusers | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/support/scripts/mkusers b/support/scripts/mkusers
index 4fc06f8fdf..d786943cf8 100755
--- a/support/scripts/mkusers
+++ b/support/scripts/mkusers
@@ -324,9 +324,9 @@ add_one_user() {
 
     # Generate a new UID if needed
     if [ ${uid} -eq ${AUTO_USER_ID} ]; then
-        uid="$( generate_uid "${username}" $FIRST_USER_GID $LAST_USER_GID )"
+        uid="$( generate_uid "${username}" $FIRST_USER_UID $LAST_USER_UID )"
     elif [ ${uid} -eq ${AUTO_SYSTEM_ID} ]; then
-        uid="$( generate_uid "${username}" $FIRST_SYSTEM_GID $LAST_SYSTEM_GID )"
+        uid="$( generate_uid "${username}" $FIRST_SYSTEM_UID $LAST_SYSTEM_UID )"
     fi
 
     # Remove any previous instance of this user
-- 
2.44.0

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Buildroot] [PATCH 0/5] support: accept text files with missing terminating \n (branch yem/newlines)
@ 2024-03-09 23:44 Yann E. MORIN
  2024-03-09 23:44 ` [Buildroot] [PATCH 1/5] support/scripts/mkusers: fix UID/GID confusion Yann E. MORIN
                   ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: Yann E. MORIN @ 2024-03-09 23:44 UTC (permalink / raw)
  To: buildroot; +Cc: Norbert Lange, Yann E . MORIN, masonwardle

Hello All!

As explained in the last commit, we expect text files to be properly
terminated with a full line, i.e. one having a final \n.

However, a lot of people do not have proper text editors, and those do
not add the terminating newline, and do not honour our .editorconfig.

This series fixes two of the affected scripts, and also fixes shellcheck
errors in both. Fixing those also uncovered an actual issue in mkusers,
so that too is fixed.

Regards,
Yann E. MORIN.


----------------------------------------------------------------
Yann E. MORIN (5):
      support/scripts/mkusers: fix UID/GID confusion
      support/scripts/mkusers: fix shellcheck errors
      support/scripts/mkusers: accept user tables without terminating \n
      support/download/check-hash: fix shellcheck errors
      support/download/check-hash: accept hash files without terminating \n

 .checkpackageignore         |  2 --
 support/download/check-hash | 11 ++++++++---
 support/scripts/mkusers     | 34 ++++++++++++++++++++++++----------
 3 files changed, 32 insertions(+), 15 deletions(-)

--
.-----------------.--------------------.------------------.--------------------.
|  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] 11+ messages in thread

* [Buildroot] [PATCH 2/5] support/scripts/mkusers: fix shellcheck errors
  2024-03-09 23:44 [Buildroot] [PATCH 0/5] support: accept text files with missing terminating \n (branch yem/newlines) Yann E. MORIN
  2024-03-09 23:44 ` [Buildroot] [PATCH 1/5] support/scripts/mkusers: fix UID/GID confusion Yann E. MORIN
@ 2024-03-09 23:44 ` Yann E. MORIN
  2024-03-09 23:44 ` [Buildroot] [PATCH 3/5] support/scripts/mkusers: accept user tables without terminating \n Yann E. MORIN
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Yann E. MORIN @ 2024-03-09 23:44 UTC (permalink / raw)
  To: buildroot; +Cc: Yann E. MORIN

the user tables do not use trailing backslash \ to continue lines,
so we don't want them to be interpreted thusly, so we use 'read -r'
(SC2162).

Integer variables need not be quoted (SC2086). In any case, should
there be an actual issue and they be set empty, that would cause a
runtime issue, wether they be quoted or not.

The binary -o and -a ar perfectly defined in bash's test (SC2166).

Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
---
 .checkpackageignore     |  1 -
 support/scripts/mkusers | 27 +++++++++++++++++++++------
 2 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/.checkpackageignore b/.checkpackageignore
index f4bfa12e1e..db09574463 100644
--- a/.checkpackageignore
+++ b/.checkpackageignore
@@ -1471,7 +1471,6 @@ support/scripts/expunge-gconv-modules Shellcheck
 support/scripts/fix-configure-powerpc64.sh EmptyLastLine
 support/scripts/generate-gitlab-ci-yml Shellcheck
 support/scripts/mkmakefile ConsecutiveEmptyLines Shellcheck
-support/scripts/mkusers Shellcheck
 support/scripts/setlocalversion Shellcheck
 support/testing/tests/core/post-build.sh Shellcheck
 support/testing/tests/package/test_opkg/post-build.sh Shellcheck
diff --git a/support/scripts/mkusers b/support/scripts/mkusers
index d786943cf8..ee09bbd1f2 100755
--- a/support/scripts/mkusers
+++ b/support/scripts/mkusers
@@ -26,6 +26,7 @@ error() {
     shift
 
     printf "%s: " "${myname}" >&2
+    # shellcheck disable=SC2059  # fmt is the format passed to error()
     printf "${fmt}" "${@}" >&2
 }
 fail() {
@@ -145,6 +146,8 @@ check_user_validity() {
         fail "invalid username '%s\n'" "${username}"
     fi
 
+    # shellcheck disable=SC2086  # gid is a non-empty int
+    # shellcheck disable=SC2166  # [ .. -o .. ] works well in this case
     if [ ${gid} -lt -2 -o ${gid} -eq 0 ]; then
         fail "invalid gid '%d' for '%s'\n" ${gid} "${username}"
     elif [ ${gid} -ge 0 ]; then
@@ -171,6 +174,8 @@ check_user_validity() {
         fi
     fi
 
+    # shellcheck disable=SC2086  # uid is a non-empty int
+    # shellcheck disable=SC2166  # [ .. -o .. ] works well in this case
     if [ ${uid} -lt -2 -o ${uid} -eq 0 ]; then
         fail "invalid uid '%d' for '%s'\n" ${uid} "${username}"
     elif [ ${uid} -ge 0 ]; then
@@ -190,6 +195,7 @@ check_user_validity() {
     fi
 
     # check the user does not already exist in another group
+    # shellcheck disable=SC2166  # [ .. -a .. ] works well in this case
     if [ -n "${_ugroup}" -a "${_ugroup}" != "${group}" ]; then
         fail "user '%s' already exists with group '%s' (wants '%s')\n" \
              "${username}" "${_ugroup}" "${group}"
@@ -218,6 +224,7 @@ generate_gid() {
                 break
             fi
         done
+        # shellcheck disable=SC2086  # gid and maxgid are non-empty ints
         if [ ${gid} -gt ${maxgid} ]; then
             fail "can not allocate a GID for group '%s'\n" "${group}"
         fi
@@ -233,6 +240,7 @@ add_one_group() {
     local members
 
     # Generate a new GID if needed
+    # shellcheck disable=SC2086  # gid is a non-empty int
     if [ ${gid} -eq ${AUTO_USER_ID} ]; then
         gid="$( generate_gid "${group}" $FIRST_USER_GID $LAST_USER_GID )"
     elif [ ${gid} -eq ${AUTO_SYSTEM_ID} ]; then
@@ -272,6 +280,7 @@ generate_uid() {
                 break
             fi
         done
+        # shellcheck disable=SC2086  # uid is a non-empty int
         if [ ${uid} -gt ${maxuid} ]; then
             fail "can not allocate a UID for user '%s'\n" "${username}"
         fi
@@ -323,6 +332,7 @@ add_one_user() {
     check_user_validity "${username}" "${uid}" "${group}" "${gid}"
 
     # Generate a new UID if needed
+    # shellcheck disable=SC2086  # uid is a non-empty int
     if [ ${uid} -eq ${AUTO_USER_ID} ]; then
         uid="$( generate_uid "${username}" $FIRST_USER_UID $LAST_USER_UID )"
     elif [ ${uid} -eq ${AUTO_SYSTEM_ID} ]; then
@@ -399,7 +409,7 @@ main() {
     fi
 
     # Read in all the file in memory, exclude empty lines and comments
-    while read line; do
+    while read -r line; do
         ENTRIES+=( "${line}" )
     done < <( sed -r -e 's/#.*//; /^[[:space:]]*$/d;' "${USERS_TABLE}" )
 
@@ -410,14 +420,16 @@ main() {
 
     # First, create all the main groups which gid is *not* automatic
     for line in "${ENTRIES[@]}"; do
-        read username uid group gid passwd home shell groups comment <<<"${line}"
+        read -r username uid group gid passwd home shell groups comment <<<"${line}"
+        # shellcheck disable=SC2086  # gid is a non-empty int
         [ ${gid} -ge 0 ] || continue    # Automatic gid
         add_one_group "${group}" "${gid}"
     done
 
     # Then, create all the main groups which gid *is* automatic
     for line in "${ENTRIES[@]}"; do
-        read username uid group gid passwd home shell groups comment <<<"${line}"
+        read -r username uid group gid passwd home shell groups comment <<<"${line}"
+        # shellcheck disable=SC2086  # gid is a non-empty int
         [ ${gid} -lt 0 ] || continue    # Non-automatic gid
         add_one_group "${group}" "${gid}"
     done
@@ -428,8 +440,9 @@ main() {
     # system gid if the uid is a system user (<= LAST_SYSTEM_UID),
     # otherwise a user gid.
     for line in "${ENTRIES[@]}"; do
-        read username uid group gid passwd home shell groups comment <<<"${line}"
+        read -r username uid group gid passwd home shell groups comment <<<"${line}"
         if [ "${groups}" != "-" ]; then
+            # shellcheck disable=SC2086  # uid is a non-empty int
             if [ ${uid} -le 0 ]; then
                 auto_id=${uid}
             elif [ ${uid} -le ${LAST_SYSTEM_UID} ]; then
@@ -450,8 +463,9 @@ main() {
 
     # Now, add users whose uid is *not* automatic
     for line in "${ENTRIES[@]}"; do
-        read username uid group gid passwd home shell groups comment <<<"${line}"
+        read -r username uid group gid passwd home shell groups comment <<<"${line}"
         [ "${username}" != "-" ] || continue # Magic string to skip user creation
+        # shellcheck disable=SC2086  # uid is a non-empty int
         [ ${uid} -ge 0         ] || continue # Automatic uid
         add_one_user "${username}" "${uid}" "${group}" "${gid}" "${passwd}" \
                      "${home}" "${shell}" "${groups}" "${comment}"
@@ -459,8 +473,9 @@ main() {
 
     # Finally, add users whose uid *is* automatic
     for line in "${ENTRIES[@]}"; do
-        read username uid group gid passwd home shell groups comment <<<"${line}"
+        read -r username uid group gid passwd home shell groups comment <<<"${line}"
         [ "${username}" != "-" ] || continue # Magic string to skip user creation
+        # shellcheck disable=SC2086  # uid is a non-empty int
         [ ${uid} -lt 0        ] || continue # Non-automatic uid
         add_one_user "${username}" "${uid}" "${group}" "${gid}" "${passwd}" \
                      "${home}" "${shell}" "${groups}" "${comment}"
-- 
2.44.0

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Buildroot] [PATCH 3/5] support/scripts/mkusers: accept user tables without terminating \n
  2024-03-09 23:44 [Buildroot] [PATCH 0/5] support: accept text files with missing terminating \n (branch yem/newlines) Yann E. MORIN
  2024-03-09 23:44 ` [Buildroot] [PATCH 1/5] support/scripts/mkusers: fix UID/GID confusion Yann E. MORIN
  2024-03-09 23:44 ` [Buildroot] [PATCH 2/5] support/scripts/mkusers: fix shellcheck errors Yann E. MORIN
@ 2024-03-09 23:44 ` Yann E. MORIN
  2024-07-14 20:50   ` Arnout Vandecappelle via buildroot
  2024-08-20 17:18   ` Peter Korsgaard
  2024-03-09 23:44 ` [Buildroot] [PATCH 4/5] support/download/check-hash: fix shellcheck errors Yann E. MORIN
                   ` (2 subsequent siblings)
  5 siblings, 2 replies; 11+ messages in thread
From: Yann E. MORIN @ 2024-03-09 23:44 UTC (permalink / raw)
  To: buildroot; +Cc: Yann E. MORIN

Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
---
 support/scripts/mkusers | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/support/scripts/mkusers b/support/scripts/mkusers
index ee09bbd1f2..08f3344518 100755
--- a/support/scripts/mkusers
+++ b/support/scripts/mkusers
@@ -409,9 +409,8 @@ main() {
     fi
 
     # Read in all the file in memory, exclude empty lines and comments
-    while read -r line; do
-        ENTRIES+=( "${line}" )
-    done < <( sed -r -e 's/#.*//; /^[[:space:]]*$/d;' "${USERS_TABLE}" )
+    # mapfile reads all lines, even the last one if it is missing a \n
+    mapfile -t ENTRIES < <( sed -r -e 's/#.*//; /^[[:space:]]*$/d;' "${USERS_TABLE}" )
 
     # We first create groups whose gid is positive, and then we create groups
     # whose gid is automatic, so that, if a group is defined both with
-- 
2.44.0

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Buildroot] [PATCH 4/5] support/download/check-hash: fix shellcheck errors
  2024-03-09 23:44 [Buildroot] [PATCH 0/5] support: accept text files with missing terminating \n (branch yem/newlines) Yann E. MORIN
                   ` (2 preceding siblings ...)
  2024-03-09 23:44 ` [Buildroot] [PATCH 3/5] support/scripts/mkusers: accept user tables without terminating \n Yann E. MORIN
@ 2024-03-09 23:44 ` Yann E. MORIN
  2024-03-09 23:44 ` [Buildroot] [PATCH 5/5] support/download/check-hash: accept hash files without terminating \n Yann E. MORIN
  2024-04-01 18:49 ` [Buildroot] [PATCH 0/5] support: accept text files with missing terminating \n (branch yem/newlines) Yann E. MORIN
  5 siblings, 0 replies; 11+ messages in thread
From: Yann E. MORIN @ 2024-03-09 23:44 UTC (permalink / raw)
  To: buildroot; +Cc: Yann E. MORIN

The hash files do not use trailing backslash \ to continue lines, so
we don't want them to be interpreted thusly, so we use 'read -r'
(SC2162).

The h_file is used twice in the same loop, once for reading from it,
and once just to print it, so there is no conflict (SC2094).

Integrer variables need not be quoted (SC2086). In any case, should
there be an actual issue and they be set empty, that would cause a
runtime issue, wether they be quoted or not.

Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
---
 .checkpackageignore         | 1 -
 support/download/check-hash | 8 ++++++--
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/.checkpackageignore b/.checkpackageignore
index db09574463..e9dcc56d32 100644
--- a/.checkpackageignore
+++ b/.checkpackageignore
@@ -1448,7 +1448,6 @@ support/dependencies/check-host-xzcat.sh Shellcheck
 support/dependencies/dependencies.sh Shellcheck
 support/download/bzr ConsecutiveEmptyLines Shellcheck
 support/download/cargo-post-process Shellcheck
-support/download/check-hash Shellcheck
 support/download/cvs Shellcheck
 support/download/dl-wrapper Shellcheck
 support/download/file Shellcheck
diff --git a/support/download/check-hash b/support/download/check-hash
index 03a6557187..9db647885a 100755
--- a/support/download/check-hash
+++ b/support/download/check-hash
@@ -57,7 +57,7 @@ check_one_hash() {
     esac
 
     # Do the hashes match?
-    _hash=$( ${_h}sum "${_file}" |cut -d ' ' -f 1 )
+    _hash="$( "${_h}sum" "${_file}" |cut -d ' ' -f 1 )"
     if [ "${_hash}" = "${_known}" ]; then
         printf "%s: OK (%s: %s)\n" "${base}" "${_h}" "${_hash}"
         return 0
@@ -78,7 +78,8 @@ nb_checks=0
 for h_file in "${h_files[@]}"; do
     [ -f "${h_file}" ] || continue
     : $((nb_h_files++))
-    while read t h f; do
+    # shellcheck disable=SC2094  # we're really reading it only once
+    while read -r t h f; do
         case "${t}" in
             ''|'#'*)
                 # Skip comments and empty lines
@@ -86,6 +87,7 @@ for h_file in "${h_files[@]}"; do
                 ;;
             *)
                 if [ "${f}" = "${base}" ]; then
+                    # shellcheck disable=SC2094  # we're only printing the h_file filename
                     check_one_hash "${t}" "${h}" "${file}" "${h_file}"
                     : $((nb_checks++))
                 fi
@@ -94,11 +96,13 @@ for h_file in "${h_files[@]}"; do
     done <"${h_file}"
 done
 
+# shellcheck disable=SC2086  # nb_h_files is a non-empty int
 if [ ${nb_h_files} -eq 0 ]; then
     printf "WARNING: no hash file for %s\n" "${base}" >&2
     exit 0
 fi
 
+# shellcheck disable=SC2086  # nb_checks is a non-empty int
 if [ ${nb_checks} -eq 0 ]; then
     case " ${BR_NO_CHECK_HASH_FOR} " in
     *" ${base} "*)
-- 
2.44.0

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Buildroot] [PATCH 5/5] support/download/check-hash: accept hash files without terminating \n
  2024-03-09 23:44 [Buildroot] [PATCH 0/5] support: accept text files with missing terminating \n (branch yem/newlines) Yann E. MORIN
                   ` (3 preceding siblings ...)
  2024-03-09 23:44 ` [Buildroot] [PATCH 4/5] support/download/check-hash: fix shellcheck errors Yann E. MORIN
@ 2024-03-09 23:44 ` Yann E. MORIN
  2024-07-14 20:50   ` Arnout Vandecappelle via buildroot
  2024-08-20 17:20   ` Peter Korsgaard
  2024-04-01 18:49 ` [Buildroot] [PATCH 0/5] support: accept text files with missing terminating \n (branch yem/newlines) Yann E. MORIN
  5 siblings, 2 replies; 11+ messages in thread
From: Yann E. MORIN @ 2024-03-09 23:44 UTC (permalink / raw)
  To: buildroot; +Cc: Yann E. MORIN, masonwardle

Lots of people are using broken text editors that 1. do not naturally
terminate text files with a final \n as is customary in UNIX text files,
and 2. do not respect our .editorconfig settings, which explicitly
require adding that final newline. See this nice summary of what a text
file is (with references to applicable standards):

    https://stackoverflow.com/questions/12916352/shell-script-read-missing-last-line/12916758#12916758

So, it is not surprising that read does not read the last "line" of a
file, when said "line" does not end with a newline, because it is thus
not really a line.

Even though we do mandate actual text files, let's be a little bit lax
in this respect, because people may write packages, and their hash
files, in a br2-external tree, and they may not have our .editorconfig
in the directory heierarchy (e.g. if buildroot is a submodule of their
br2-external tree, or whatever).

mapfile does not suffer from this limitation, though, and correctly
reads all lines from a file, even the final line-that-is-not-a-line.

mapfile was introduced in bash 4.0, released on 2009-01-20, more than
15 years ago. Debian squeeze, released in 2011 already had bash 4.1.
Those are really ancient. So, it means we can indeed expect bash
version 4.0 or later; which means mapfile is available.

"It should be fine!"

Fixes: #15976

Reported-by: masonwardle@gmail.com
Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
---
 support/download/check-hash | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/support/download/check-hash b/support/download/check-hash
index 9db647885a..d18ec8b134 100755
--- a/support/download/check-hash
+++ b/support/download/check-hash
@@ -78,8 +78,10 @@ nb_checks=0
 for h_file in "${h_files[@]}"; do
     [ -f "${h_file}" ] || continue
     : $((nb_h_files++))
-    # shellcheck disable=SC2094  # we're really reading it only once
-    while read -r t h f; do
+    # mapfile reads all lines, even the last one if it is missing a \n
+    mapfile -t hash_lines <"${h_file}"
+    for hash_line in "${hash_lines[@]}"; do
+        read -r t h f <<<"${hash_line}"
         case "${t}" in
             ''|'#'*)
                 # Skip comments and empty lines
@@ -87,13 +89,12 @@ for h_file in "${h_files[@]}"; do
                 ;;
             *)
                 if [ "${f}" = "${base}" ]; then
-                    # shellcheck disable=SC2094  # we're only printing the h_file filename
                     check_one_hash "${t}" "${h}" "${file}" "${h_file}"
                     : $((nb_checks++))
                 fi
                 ;;
         esac
-    done <"${h_file}"
+    done
 done
 
 # shellcheck disable=SC2086  # nb_h_files is a non-empty int
-- 
2.44.0

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [Buildroot] [PATCH 0/5] support: accept text files with missing terminating \n (branch yem/newlines)
  2024-03-09 23:44 [Buildroot] [PATCH 0/5] support: accept text files with missing terminating \n (branch yem/newlines) Yann E. MORIN
                   ` (4 preceding siblings ...)
  2024-03-09 23:44 ` [Buildroot] [PATCH 5/5] support/download/check-hash: accept hash files without terminating \n Yann E. MORIN
@ 2024-04-01 18:49 ` Yann E. MORIN
  5 siblings, 0 replies; 11+ messages in thread
From: Yann E. MORIN @ 2024-04-01 18:49 UTC (permalink / raw)
  To: buildroot; +Cc: Norbert Lange, masonwardle

All,

On 2024-03-10 00:44 +0100, Yann E. MORIN spake thusly:
> This series fixes two of the affected scripts, and also fixes shellcheck
> errors in both. Fixing those also uncovered an actual issue in mkusers,
> so that too is fixed.
> 
> Yann E. MORIN (5):
>       support/scripts/mkusers: fix UID/GID confusion
>       support/scripts/mkusers: fix shellcheck errors
>       support/download/check-hash: fix shellcheck errors

Those three: applied to master.

>       support/scripts/mkusers: accept user tables without terminating \n
>       support/download/check-hash: accept hash files without terminating \n

Those two: hold for a little while yet...

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] 11+ messages in thread

* Re: [Buildroot] [PATCH 3/5] support/scripts/mkusers: accept user tables without terminating \n
  2024-03-09 23:44 ` [Buildroot] [PATCH 3/5] support/scripts/mkusers: accept user tables without terminating \n Yann E. MORIN
@ 2024-07-14 20:50   ` Arnout Vandecappelle via buildroot
  2024-08-20 17:18   ` Peter Korsgaard
  1 sibling, 0 replies; 11+ messages in thread
From: Arnout Vandecappelle via buildroot @ 2024-07-14 20:50 UTC (permalink / raw)
  To: Yann E. MORIN, buildroot



On 10/03/2024 00:44, Yann E. MORIN wrote:
> Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>

  Applied to master, thanks.

> ---
>   support/scripts/mkusers | 5 ++---
>   1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/support/scripts/mkusers b/support/scripts/mkusers
> index ee09bbd1f2..08f3344518 100755
> --- a/support/scripts/mkusers
> +++ b/support/scripts/mkusers
> @@ -409,9 +409,8 @@ main() {
>       fi
>   
>       # Read in all the file in memory, exclude empty lines and comments
> -    while read -r line; do
> -        ENTRIES+=( "${line}" )
> -    done < <( sed -r -e 's/#.*//; /^[[:space:]]*$/d;' "${USERS_TABLE}" )
> +    # mapfile reads all lines, even the last one if it is missing a \n

  I don't think this comment is very useful (if you wonder about why mapfile is 
used instead of read, you can check the git history). But I was too lazy ro 
remove it :-)

  Regards,
  Arnout

> +    mapfile -t ENTRIES < <( sed -r -e 's/#.*//; /^[[:space:]]*$/d;' "${USERS_TABLE}" )
>   
>       # We first create groups whose gid is positive, and then we create groups
>       # whose gid is automatic, so that, if a group is defined both with
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Buildroot] [PATCH 5/5] support/download/check-hash: accept hash files without terminating \n
  2024-03-09 23:44 ` [Buildroot] [PATCH 5/5] support/download/check-hash: accept hash files without terminating \n Yann E. MORIN
@ 2024-07-14 20:50   ` Arnout Vandecappelle via buildroot
  2024-08-20 17:20   ` Peter Korsgaard
  1 sibling, 0 replies; 11+ messages in thread
From: Arnout Vandecappelle via buildroot @ 2024-07-14 20:50 UTC (permalink / raw)
  To: Yann E. MORIN, buildroot; +Cc: masonwardle



On 10/03/2024 00:44, Yann E. MORIN wrote:
> Lots of people are using broken text editors that 1. do not naturally
> terminate text files with a final \n as is customary in UNIX text files,
> and 2. do not respect our .editorconfig settings, which explicitly
> require adding that final newline. See this nice summary of what a text
> file is (with references to applicable standards):
> 
>      https://stackoverflow.com/questions/12916352/shell-script-read-missing-last-line/12916758#12916758
> 
> So, it is not surprising that read does not read the last "line" of a
> file, when said "line" does not end with a newline, because it is thus
> not really a line.
> 
> Even though we do mandate actual text files, let's be a little bit lax
> in this respect, because people may write packages, and their hash
> files, in a br2-external tree, and they may not have our .editorconfig
> in the directory heierarchy (e.g. if buildroot is a submodule of their
> br2-external tree, or whatever).
> 
> mapfile does not suffer from this limitation, though, and correctly
> reads all lines from a file, even the final line-that-is-not-a-line.
> 
> mapfile was introduced in bash 4.0, released on 2009-01-20, more than
> 15 years ago. Debian squeeze, released in 2011 already had bash 4.1.
> Those are really ancient. So, it means we can indeed expect bash
> version 4.0 or later; which means mapfile is available.
> 
> "It should be fine!"
> 
> Fixes: #15976
> 
> Reported-by: masonwardle@gmail.com
> Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>

  Applied to master, thanks.

  Regards,
  Arnout

> ---
>   support/download/check-hash | 9 +++++----
>   1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/support/download/check-hash b/support/download/check-hash
> index 9db647885a..d18ec8b134 100755
> --- a/support/download/check-hash
> +++ b/support/download/check-hash
> @@ -78,8 +78,10 @@ nb_checks=0
>   for h_file in "${h_files[@]}"; do
>       [ -f "${h_file}" ] || continue
>       : $((nb_h_files++))
> -    # shellcheck disable=SC2094  # we're really reading it only once
> -    while read -r t h f; do
> +    # mapfile reads all lines, even the last one if it is missing a \n
> +    mapfile -t hash_lines <"${h_file}"
> +    for hash_line in "${hash_lines[@]}"; do
> +        read -r t h f <<<"${hash_line}"
>           case "${t}" in
>               ''|'#'*)
>                   # Skip comments and empty lines
> @@ -87,13 +89,12 @@ for h_file in "${h_files[@]}"; do
>                   ;;
>               *)
>                   if [ "${f}" = "${base}" ]; then
> -                    # shellcheck disable=SC2094  # we're only printing the h_file filename
>                       check_one_hash "${t}" "${h}" "${file}" "${h_file}"
>                       : $((nb_checks++))
>                   fi
>                   ;;
>           esac
> -    done <"${h_file}"
> +    done
>   done
>   
>   # shellcheck disable=SC2086  # nb_h_files is a non-empty int
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Buildroot] [PATCH 3/5] support/scripts/mkusers: accept user tables without terminating \n
  2024-03-09 23:44 ` [Buildroot] [PATCH 3/5] support/scripts/mkusers: accept user tables without terminating \n Yann E. MORIN
  2024-07-14 20:50   ` Arnout Vandecappelle via buildroot
@ 2024-08-20 17:18   ` Peter Korsgaard
  1 sibling, 0 replies; 11+ messages in thread
From: Peter Korsgaard @ 2024-08-20 17:18 UTC (permalink / raw)
  To: Yann E. MORIN; +Cc: buildroot

>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:

 > Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>

Committed to 2024.02.x and 2024.05.x, thanks.

-- 
Bye, Peter Korsgaard
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Buildroot] [PATCH 5/5] support/download/check-hash: accept hash files without terminating \n
  2024-03-09 23:44 ` [Buildroot] [PATCH 5/5] support/download/check-hash: accept hash files without terminating \n Yann E. MORIN
  2024-07-14 20:50   ` Arnout Vandecappelle via buildroot
@ 2024-08-20 17:20   ` Peter Korsgaard
  1 sibling, 0 replies; 11+ messages in thread
From: Peter Korsgaard @ 2024-08-20 17:20 UTC (permalink / raw)
  To: Yann E. MORIN; +Cc: masonwardle, buildroot

>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:

 > Lots of people are using broken text editors that 1. do not naturally
 > terminate text files with a final \n as is customary in UNIX text files,
 > and 2. do not respect our .editorconfig settings, which explicitly
 > require adding that final newline. See this nice summary of what a text
 > file is (with references to applicable standards):

 >     https://stackoverflow.com/questions/12916352/shell-script-read-missing-last-line/12916758#12916758

 > So, it is not surprising that read does not read the last "line" of a
 > file, when said "line" does not end with a newline, because it is thus
 > not really a line.

 > Even though we do mandate actual text files, let's be a little bit lax
 > in this respect, because people may write packages, and their hash
 > files, in a br2-external tree, and they may not have our .editorconfig
 > in the directory heierarchy (e.g. if buildroot is a submodule of their
 > br2-external tree, or whatever).

 > mapfile does not suffer from this limitation, though, and correctly
 > reads all lines from a file, even the final line-that-is-not-a-line.

 > mapfile was introduced in bash 4.0, released on 2009-01-20, more than
 > 15 years ago. Debian squeeze, released in 2011 already had bash 4.1.
 > Those are really ancient. So, it means we can indeed expect bash
 > version 4.0 or later; which means mapfile is available.

 > "It should be fine!"

 > Fixes: #15976

 > Reported-by: masonwardle@gmail.com
 > Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>

Committed to 2024.02.x and 2024.05.x, thanks.

-- 
Bye, Peter Korsgaard
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2024-08-20 17:20 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-09 23:44 [Buildroot] [PATCH 0/5] support: accept text files with missing terminating \n (branch yem/newlines) Yann E. MORIN
2024-03-09 23:44 ` [Buildroot] [PATCH 1/5] support/scripts/mkusers: fix UID/GID confusion Yann E. MORIN
2024-03-09 23:44 ` [Buildroot] [PATCH 2/5] support/scripts/mkusers: fix shellcheck errors Yann E. MORIN
2024-03-09 23:44 ` [Buildroot] [PATCH 3/5] support/scripts/mkusers: accept user tables without terminating \n Yann E. MORIN
2024-07-14 20:50   ` Arnout Vandecappelle via buildroot
2024-08-20 17:18   ` Peter Korsgaard
2024-03-09 23:44 ` [Buildroot] [PATCH 4/5] support/download/check-hash: fix shellcheck errors Yann E. MORIN
2024-03-09 23:44 ` [Buildroot] [PATCH 5/5] support/download/check-hash: accept hash files without terminating \n Yann E. MORIN
2024-07-14 20:50   ` Arnout Vandecappelle via buildroot
2024-08-20 17:20   ` Peter Korsgaard
2024-04-01 18:49 ` [Buildroot] [PATCH 0/5] support: accept text files with missing terminating \n (branch yem/newlines) Yann E. MORIN

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox