public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v3 0/2] Standardize the sorting of Makefile variables
@ 2023-05-10 11:58 Carlos O'Donell
  2023-05-10 11:58 ` [PATCH v3 1/2] scripts: Add sort-makefile-lines.py to sort " Carlos O'Donell
  2023-05-10 11:58 ` [PATCH v3 2/2] nptl: Reformat Makefile Carlos O'Donell
  0 siblings, 2 replies; 12+ messages in thread
From: Carlos O'Donell @ 2023-05-10 11:58 UTC (permalink / raw)
  To: libc-alpha, siddhesh, alx.manpages; +Cc: Carlos O'Donell

The project has started to re-flow and sort Makefile variables to make
it easier to manage the lists of inputs and tests.  While splitting the
lines is easy to do, the sorting is more onerous because of the desire
to sort like tests with like tests.  The following is an attempt to
standardize teh sorting following the consensus discussion on the list
which suggested using LC_COLLATE=C and sorting similar tests in
numerical order of their prefix.

The series re-flows and then sorts sysdeps/pthread/Makefile using the
new python helper script.

Carlos O'Donell (2):
  scripts: Add sort-makefile-lines.py to sort Makefile variables.
  nptl: Reformat Makefile.

 scripts/sort-makefile-lines.py                | 160 +++++++
 sysdeps/pthread/Makefile                      | 433 +++++++++++++-----
 .../{tst-mutex7robust.c => tst-robust11.c}    |   0
 3 files changed, 480 insertions(+), 113 deletions(-)
 create mode 100755 scripts/sort-makefile-lines.py
 rename sysdeps/pthread/{tst-mutex7robust.c => tst-robust11.c} (100%)

-- 
2.39.2


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

* [PATCH v3 1/2] scripts: Add sort-makefile-lines.py to sort Makefile variables.
  2023-05-10 11:58 [PATCH v3 0/2] Standardize the sorting of Makefile variables Carlos O'Donell
@ 2023-05-10 11:58 ` Carlos O'Donell
  2023-05-10 12:42   ` Alejandro Colomar
  2023-05-10 15:49   ` Siddhesh Poyarekar
  2023-05-10 11:58 ` [PATCH v3 2/2] nptl: Reformat Makefile Carlos O'Donell
  1 sibling, 2 replies; 12+ messages in thread
From: Carlos O'Donell @ 2023-05-10 11:58 UTC (permalink / raw)
  To: libc-alpha, siddhesh, alx.manpages; +Cc: Carlos O'Donell

The scripts/sort-makefile-lines.py script sorts Makefile variables
according to project expected order.

The script can be used like this:

$ scripts/sort-makefile-lines.py < elf/Makefile > elf/Makefile.tmp
$ mv elf/Makefile.tmp elf/Makefile
---
v2->v3: Use stdin/stdout.

 scripts/sort-makefile-lines.py | 160 +++++++++++++++++++++++++++++++++
 1 file changed, 160 insertions(+)
 create mode 100755 scripts/sort-makefile-lines.py

diff --git a/scripts/sort-makefile-lines.py b/scripts/sort-makefile-lines.py
new file mode 100755
index 0000000000..fd657df970
--- /dev/null
+++ b/scripts/sort-makefile-lines.py
@@ -0,0 +1,160 @@
+#!/usr/bin/python3
+# Sort Makefile lines as expected by project policy.
+# Copyright (C) 2023 Free Software Foundation, Inc.
+# This file is part of the GNU C Library.
+#
+# The GNU C Library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+#
+# The GNU C Library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with the GNU C Library; if not, see
+# <https://www.gnu.org/licenses/>.
+
+# The project consensus is to split Makefile variable assignment
+# across multiple lines with one value per line.  The values are
+# then sorted as described below, and terminated with a special
+# list termination marker.  This splitting makes it much easier
+# to add new tests to the list since they become just a single
+# line insertion.  It also makes backports and merges easier
+# since the new test may not conflict due to the ordering.
+#
+# Consensus discussion:
+# https://inbox.sourceware.org/libc-alpha/f6406204-84f5-adb1-d00e-979ebeebbbde@redhat.com/
+#
+# To support cleaning up Makefiles we created this program to
+# help sort existing lists converted to the new format.
+#
+# The program takes as input the Makefile to sort correctly,
+# and the output file to write the correctly sorted output
+# (it can be the same file).
+#
+# Sorting is only carried out between two special markers:
+# (a) Marker start is '<variable> += \' (or '= \', or ':= \')
+# (b) Marker end is '  # <variable>' (whitespace matters)
+# With everthing between (a) and (b) being sorted accordingly.
+#
+# You can use it like this:
+# $ scripts/sort-makefile-lines.py < elf/Makefile > elf/Makefile.tmp
+# $ mv elf/Makefile.tmp elf/Makefile
+#
+# The Makefile lines in the project are sorted using the
+# following rules:
+# - All lines are sorted as-if `LC_COLLATE=C sort`
+# - Lines that have a numeric suffix and whose leading prefix
+#   matches exactly are sorted according the numeric suffix
+#   in increasing numerical order.
+#
+# For example:
+# ~~~
+# tests += \
+#   test-a \
+#   test-b \
+#   test-b1 \
+#   test-b2 \
+#   test-b10 \
+#   test-b20 \
+#   test-b100 \
+#   # tests
+# ~~~
+# This example shows tests sorted alphabetically, followed
+# by a numeric suffix sort in increasing numeric order.
+#
+# Cleanups:
+# - Tests that end in "a" or "b" variants should be renamed to
+#   end in just the numerical value. For example 'tst-mutex7robust'
+#   should be renamed to 'tst-mutex12' (the highest numbered test)
+#   or 'tst-robust11' (the highest numbered test) in order to get
+#   reasonable ordering.
+# - Modules that end in "mod" or "mod1" should be renamed. For
+#   example 'tst-atfork2mod' should be renamed to 'tst-mod-atfork2'
+#   (test module for atfork2). If there are more than one module
+#   then they should be named with a suffix that uses [0-9] first
+#   then [A-Z] next for a total of 36 possible modules per test.
+#   No manually listed test currently uses more than that (though
+#   automatically generated tests may; they don't need sorting).
+# - Avoid including another test and instead refactor into common
+#   code with all tests including hte common code, then give the
+#   tests unique names.
+#
+# If you have a Makefile that needs converting, then you can
+# quickly split the values into one-per-line, ensure the start
+# and end markers are in place, and then run the script to
+# sort the values.
+
+import sys
+import locale
+import re
+import functools
+
+def glibc_makefile_numeric(string1, string2):
+    # Check if string1 has a numeric suffix.
+    var1 = re.search(r'([0-9]+) \\$', string1)
+    var2 = re.search(r'([0-9]+) \\$', string2)
+    if var1 and var2:
+        if string1[0:var1.span()[0]] == string2[0:var2.span()[0]]:
+            # string1 and string2 both share a prefix and
+            # have a numeric suffix that can be compared.
+            # Sort order is based on the numeric suffix.
+            return int(var1.group(1)) > int(var2.group(1))
+    # Default to strcoll.
+    return locale.strcoll(string1, string2)
+
+def sort_lines(lines):
+
+    # Use the C locale for language independent collation.
+    locale.setlocale (locale.LC_ALL, "C")
+
+    # Sort using a glibc-specific sorting function.
+    lines = sorted(lines, key=functools.cmp_to_key(glibc_makefile_numeric))
+
+    return lines
+
+def sort_makefile_lines():
+
+    # Read the whole Makefile.
+    lines = sys.stdin.readlines()
+
+    # Build a list of all start markers (tuple includes name).
+    startmarks = []
+    for i in range(len(lines)):
+        # Look for things like "var = \", "var := \" or "var += \"
+        # to start the sorted list.
+        var = re.search(r'^([a-zA-Z0-9-]*) [\+:]?\= \\$', lines[i])
+        if var:
+            # Remember the index and the name.
+            startmarks.append((i, var.group(1)))
+
+    # For each start marker try to find a matching end mark
+    # and build a block that needs sorting.  The end marker
+    # must have the matching comment name for it to be valid.
+    rangemarks = []
+    for sm in startmarks:
+        # Look for things like "  # var" to end the sorted list.
+        reg = r'^  # ' + sm[1] + r'$'
+        for j in range(sm[0] + 1, len(lines)):
+            if re.search(reg, lines[j]):
+                # Rembember the block to sort (inclusive).
+                rangemarks.append((sm[0] + 1, j))
+                break
+
+    # We now have a list of all ranges that need sorting.
+    # Sort those ranges (inclusive).
+    for r in rangemarks:
+        lines[r[0]:r[1]] = sort_lines(lines[r[0]:r[1]])
+
+    # Output the whole list with sorted lines to stdout.
+    [sys.stdout.write(line) for line in lines]
+
+
+def main(argv):
+    sort_makefile_lines ()
+
+if __name__ == '__main__':
+    main(sys.argv[1:])
-- 
2.39.2


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

* [PATCH v3 2/2] nptl: Reformat Makefile.
  2023-05-10 11:58 [PATCH v3 0/2] Standardize the sorting of Makefile variables Carlos O'Donell
  2023-05-10 11:58 ` [PATCH v3 1/2] scripts: Add sort-makefile-lines.py to sort " Carlos O'Donell
@ 2023-05-10 11:58 ` Carlos O'Donell
  2023-05-10 13:01   ` Siddhesh Poyarekar
  1 sibling, 1 reply; 12+ messages in thread
From: Carlos O'Donell @ 2023-05-10 11:58 UTC (permalink / raw)
  To: libc-alpha, siddhesh, alx.manpages; +Cc: Carlos O'Donell

Reflow all long lines adding comment terminators.
Sort all reflowed text using scripts/sort-makefile-lines.py.

No code generation changes observed in binary artifacts.
No regressions on x86_64 and i686.
---
 sysdeps/pthread/Makefile                      | 433 +++++++++++++-----
 .../{tst-mutex7robust.c => tst-robust11.c}    |   0
 2 files changed, 320 insertions(+), 113 deletions(-)
 rename sysdeps/pthread/{tst-mutex7robust.c => tst-robust11.c} (100%)

diff --git a/sysdeps/pthread/Makefile b/sysdeps/pthread/Makefile
index c2f5588bd9..5df1109dd3 100644
--- a/sysdeps/pthread/Makefile
+++ b/sysdeps/pthread/Makefile
@@ -21,9 +21,17 @@ $(objpfx)tst-timer: $(librt)
 endif
 
 ifneq (,$(filter $(subdir),htl nptl))
-headers += threads.h
-
-routines += thrd_current thrd_equal thrd_sleep thrd_yield pthread_atfork
+headers += \
+  threads.h \
+  # headers
+
+routines += \
+  pthread_atfork \
+  thrd_current \
+  thrd_equal \
+  thrd_sleep \
+  thrd_yield \
+  # routines
 
 $(libpthread-routines-var) += \
   call_once \
@@ -48,86 +56,231 @@ $(libpthread-routines-var) += \
   tss_delete \
   tss_get \
   tss_set \
+  # $(libpthread-routines-var)
 
-tests += tst-cnd-basic tst-mtx-trylock tst-cnd-broadcast \
-	 tst-cnd-timedwait tst-thrd-detach tst-mtx-basic tst-thrd-sleep \
-	 tst-mtx-recursive tst-tss-basic tst-call-once tst-mtx-timedlock \
-	 \
-	 tst-abstime \
-	 tst-pt-align tst-pt-align3 \
-	 tst-attr1 \
-	 tst-backtrace1 \
-	 tst-bad-schedattr \
-	 tst-barrier1 tst-barrier2 tst-barrier3 tst-barrier4 \
-	 tst-basic1 tst-basic2 tst-basic3 tst-basic4 tst-basic5 tst-basic6 \
-	 tst-basic7 \
-	 tst-cancel-self tst-cancel-self-cancelstate \
-	 tst-cancel-self-canceltype tst-cancel-self-testcancel \
-	 tst-cancel1 tst-cancel2 tst-cancel3 \
-	 tst-cancel4 tst-cancel5 \
-	 tst-cancel6 tst-cancel8 tst-cancel9 tst-cancel10 tst-cancel11 \
-	 tst-cancel12 tst-cancel13 tst-cancel14 tst-cancel15 tst-cancel16 \
-	 tst-cancel18 tst-cancel19 tst-cancel20 tst-cancel21 \
-	 tst-cancel22 tst-cancel23 tst-cancel26 tst-cancel27 tst-cancel28 \
-	 tst-cancel29 \
-	 tst-cleanup0 tst-cleanup1 tst-cleanup2 tst-cleanup3 \
-	 tst-clock1 \
-	 tst-cond-except \
-	 tst-cond1 tst-cond2 tst-cond3 tst-cond4 tst-cond5 tst-cond6 tst-cond7 \
-	 tst-cond8 tst-cond9 tst-cond10 tst-cond11 tst-cond12 tst-cond13 \
-	 tst-cond14 tst-cond15 tst-cond16 tst-cond17 tst-cond18 tst-cond19 \
-	 tst-cond20 tst-cond21 tst-cond23 tst-cond24 tst-cond25 tst-cond27 \
-	 tst-create-detached \
-	 tst-detach1 \
-	 tst-eintr2 tst-eintr3 tst-eintr4 tst-eintr5 \
-	 tst-exec1 tst-exec2 tst-exec3 \
-	 tst-exit1 tst-exit2 tst-exit3 \
-	 tst-flock1 tst-flock2 \
-	 tst-fork1 tst-fork2 tst-fork3 tst-fork4 \
-	 tst-atfork1 \
-	 tst-getpid3 \
-	 tst-join1 tst-join2 tst-join3 tst-join4 tst-join5 tst-join6 tst-join7 \
-	 tst-join8 tst-join9 tst-join10 tst-join11 tst-join12 tst-join13 \
-	 tst-join14 tst-join15 \
-	 tst-key1 tst-key2 tst-key3 tst-key4 \
-	 tst-kill1 tst-kill2 tst-kill3 tst-kill5 tst-kill6 \
-	 tst-locale1 tst-locale2 \
-	 tst-memstream \
-	 tst-mutex-errorcheck tst-mutex1 tst-mutex2 tst-mutex3 tst-mutex4 \
-	 tst-mutex5 tst-mutex6 tst-mutex7 tst-mutex7robust tst-mutex9 \
-	 tst-mutex10 tst-mutex11 tst-pthread-mutexattr \
-	 tst-once1 tst-once2 tst-once3 tst-once4 \
-	 tst-pt-popen1 \
-	 tst-raise1 \
-	 tst-robust1 tst-robust2 tst-robust3 tst-robust4 tst-robust5 \
-	 tst-robust6 tst-robust7 tst-robust9 tst-robust10 \
-	 tst-rwlock1 tst-rwlock4 tst-rwlock5 tst-rwlock12 \
-	 tst-rwlock13 tst-rwlock14 tst-rwlock16 \
-	 tst-rwlock-tryrdlock-stall tst-rwlock-trywrlock-stall \
-	 tst-sem1 tst-sem2 tst-sem3 tst-sem4 tst-sem5 tst-sem6 tst-sem7 \
-	 tst-sem8 tst-sem9 tst-sem10 tst-sem14 tst-sem15 tst-sem16 \
-	 tst-setuid3 \
-	 tst-signal1 tst-signal2 \
-	 tst-signal4 tst-signal5 tst-signal6 tst-signal8 \
-	 tst-spin1 tst-spin2 tst-spin3 tst-spin4 \
-	 tst-stack1 \
-	 tst-stdio1 tst-stdio2 \
-	 tst-pt-sysconf \
-	 tst-pt-tls1 tst-pt-tls2 \
-	 tst-tsd1 tst-tsd2 tst-tsd5 tst-tsd6 \
-	 tst-umask1 \
-	 tst-unload \
-	 tst-unwind-thread \
-	 tst-pt-vfork1 tst-pt-vfork2 tst-vfork1x tst-vfork2x \
-	 tst-pthread-exit-signal \
-	 tst-pthread-setuid-loop \
-	 tst-pthread_cancel-exited \
-	 tst-pthread_cancel-select-loop \
-	 tst-pthread-raise-blocked-self \
-	 tst-pthread_kill-exited \
-	 tst-pthread_kill-exiting \
-	 tst-cancel30 \
-	 # tests
+tests += \
+  tst-abstime \
+  tst-atfork1 \
+  tst-attr1 \
+  tst-backtrace1 \
+  tst-bad-schedattr \
+  tst-barrier1 \
+  tst-barrier2 \
+  tst-barrier3 \
+  tst-barrier4 \
+  tst-basic1 \
+  tst-basic2 \
+  tst-basic3 \
+  tst-basic4 \
+  tst-basic5 \
+  tst-basic6 \
+  tst-basic7 \
+  tst-call-once \
+  tst-cancel-self \
+  tst-cancel-self-cancelstate \
+  tst-cancel-self-canceltype \
+  tst-cancel-self-testcancel \
+  tst-cancel1 \
+  tst-cancel2 \
+  tst-cancel3 \
+  tst-cancel4 \
+  tst-cancel5 \
+  tst-cancel6 \
+  tst-cancel8 \
+  tst-cancel9 \
+  tst-cancel10 \
+  tst-cancel11 \
+  tst-cancel12 \
+  tst-cancel13 \
+  tst-cancel14 \
+  tst-cancel15 \
+  tst-cancel16 \
+  tst-cancel18 \
+  tst-cancel19 \
+  tst-cancel20 \
+  tst-cancel21 \
+  tst-cancel22 \
+  tst-cancel23 \
+  tst-cancel26 \
+  tst-cancel27 \
+  tst-cancel28 \
+  tst-cancel29 \
+  tst-cancel30 \
+  tst-cleanup0 \
+  tst-cleanup1 \
+  tst-cleanup2 \
+  tst-cleanup3 \
+  tst-clock1 \
+  tst-cnd-basic \
+  tst-cnd-broadcast \
+  tst-cnd-timedwait \
+  tst-cond-except \
+  tst-cond1 \
+  tst-cond2 \
+  tst-cond3 \
+  tst-cond4 \
+  tst-cond5 \
+  tst-cond6 \
+  tst-cond7 \
+  tst-cond8 \
+  tst-cond9 \
+  tst-cond10 \
+  tst-cond11 \
+  tst-cond12 \
+  tst-cond13 \
+  tst-cond14 \
+  tst-cond15 \
+  tst-cond16 \
+  tst-cond17 \
+  tst-cond18 \
+  tst-cond19 \
+  tst-cond20 \
+  tst-cond21 \
+  tst-cond23 \
+  tst-cond24 \
+  tst-cond25 \
+  tst-cond27 \
+  tst-create-detached \
+  tst-detach1 \
+  tst-eintr2 \
+  tst-eintr3 \
+  tst-eintr4 \
+  tst-eintr5 \
+  tst-exec1 \
+  tst-exec2 \
+  tst-exec3 \
+  tst-exit1 \
+  tst-exit2 \
+  tst-exit3 \
+  tst-flock1 \
+  tst-flock2 \
+  tst-fork1 \
+  tst-fork2 \
+  tst-fork3 \
+  tst-fork4 \
+  tst-getpid3 \
+  tst-join1 \
+  tst-join2 \
+  tst-join3 \
+  tst-join4 \
+  tst-join5 \
+  tst-join6 \
+  tst-join7 \
+  tst-join8 \
+  tst-join9 \
+  tst-join10 \
+  tst-join11 \
+  tst-join12 \
+  tst-join13 \
+  tst-join14 \
+  tst-join15 \
+  tst-key1 \
+  tst-key2 \
+  tst-key3 \
+  tst-key4 \
+  tst-kill1 \
+  tst-kill2 \
+  tst-kill3 \
+  tst-kill5 \
+  tst-kill6 \
+  tst-locale1 \
+  tst-locale2 \
+  tst-memstream \
+  tst-mtx-basic \
+  tst-mtx-recursive \
+  tst-mtx-timedlock \
+  tst-mtx-trylock \
+  tst-mutex-errorcheck \
+  tst-mutex1 \
+  tst-mutex2 \
+  tst-mutex3 \
+  tst-mutex4 \
+  tst-mutex5 \
+  tst-mutex6 \
+  tst-mutex7 \
+  tst-mutex9 \
+  tst-mutex10 \
+  tst-mutex11 \
+  tst-once1 \
+  tst-once2 \
+  tst-once3 \
+  tst-once4 \
+  tst-pt-align \
+  tst-pt-align3 \
+  tst-pt-popen1 \
+  tst-pt-sysconf \
+  tst-pt-tls1 \
+  tst-pt-tls2 \
+  tst-pt-vfork1 \
+  tst-pt-vfork2 \
+  tst-pthread-exit-signal \
+  tst-pthread-mutexattr \
+  tst-pthread-raise-blocked-self \
+  tst-pthread-setuid-loop \
+  tst-pthread_cancel-exited \
+  tst-pthread_cancel-select-loop \
+  tst-pthread_kill-exited \
+  tst-pthread_kill-exiting \
+  tst-raise1 \
+  tst-robust1 \
+  tst-robust2 \
+  tst-robust3 \
+  tst-robust4 \
+  tst-robust5 \
+  tst-robust6 \
+  tst-robust7 \
+  tst-robust9 \
+  tst-robust10 \
+  tst-robust11 \
+  tst-rwlock-tryrdlock-stall \
+  tst-rwlock-trywrlock-stall \
+  tst-rwlock1 \
+  tst-rwlock4 \
+  tst-rwlock5 \
+  tst-rwlock12 \
+  tst-rwlock13 \
+  tst-rwlock14 \
+  tst-rwlock16 \
+  tst-sem1 \
+  tst-sem2 \
+  tst-sem3 \
+  tst-sem4 \
+  tst-sem5 \
+  tst-sem6 \
+  tst-sem7 \
+  tst-sem8 \
+  tst-sem9 \
+  tst-sem10 \
+  tst-sem14 \
+  tst-sem15 \
+  tst-sem16 \
+  tst-setuid3 \
+  tst-signal1 \
+  tst-signal2 \
+  tst-signal4 \
+  tst-signal5 \
+  tst-signal6 \
+  tst-signal8 \
+  tst-spin1 \
+  tst-spin2 \
+  tst-spin3 \
+  tst-spin4 \
+  tst-stack1 \
+  tst-stdio1 \
+  tst-stdio2 \
+  tst-thrd-detach \
+  tst-thrd-sleep \
+  tst-tsd1 \
+  tst-tsd2 \
+  tst-tsd5 \
+  tst-tsd6 \
+  tst-tss-basic \
+  tst-umask1 \
+  tst-unload \
+  tst-unwind-thread \
+  tst-vfork1x \
+  tst-vfork2x \
+  # tests
 
 tests-time64 += \
   tst-abstime-time64 \
@@ -138,47 +291,70 @@ tests-time64 += \
   tst-rwlock14-time64 \
   tst-sem5-time64 \
   tst-thrd-sleep-time64 \
+  # tests-time64
 
 static-only-routines = pthread_atfork
 
 # Files which must not be linked with libpthread.
-tests-nolibpthread += tst-unload
+tests-nolibpthread += \
+  tst-unload \
+  # tests-nolibpthread
 
 # GCC-4.9 compiles 'sprintf(NULL, ...)' into UD2 on x86_64 without -fno-builtin
 CFLAGS-tst-cleanup2.c += -fno-builtin
 CFLAGS-tst-cleanupx2.c += -fno-builtin
 
-tests += tst-cancelx2 tst-cancelx3 tst-cancelx6 tst-cancelx8 tst-cancelx9 \
-	 tst-cancelx4 tst-cancelx5 \
-	 tst-cancelx10 tst-cancelx11 tst-cancelx12 tst-cancelx13 tst-cancelx14 \
-	 tst-cancelx15 tst-cancelx16 tst-cancelx18 tst-cancelx20 tst-cancelx21 \
-	 tst-cleanupx0 tst-cleanupx1 tst-cleanupx2 tst-cleanupx3
+tests += \
+  tst-cancelx2 \
+  tst-cancelx3 \
+  tst-cancelx4 \
+  tst-cancelx5 \
+  tst-cancelx6 \
+  tst-cancelx8 \
+  tst-cancelx9 \
+  tst-cancelx10 \
+  tst-cancelx11 \
+  tst-cancelx12 \
+  tst-cancelx13 \
+  tst-cancelx14 \
+  tst-cancelx15 \
+  tst-cancelx16 \
+  tst-cancelx18 \
+  tst-cancelx20 \
+  tst-cancelx21 \
+  tst-cleanupx0 \
+  tst-cleanupx1 \
+  tst-cleanupx2 \
+  tst-cleanupx3 \
+  # tests
 
 ifeq ($(build-shared),yes)
 tests += \
-  tst-atfork2 \
-  tst-pt-tls4 \
   tst-_res1 \
-  tst-fini1 \
-  tst-create1 \
+  tst-atfork2 \
   tst-atfork3 \
   tst-atfork4 \
-# tests
+  tst-create1 \
+  tst-fini1 \
+  tst-pt-tls4 \
+  # tests
 
-tests-nolibpthread += tst-fini1
+tests-nolibpthread += \
+  tst-fini1 \
+  # tests-nolibpthread
 endif
 
 modules-names += \
-  tst-atfork2mod \
-  tst-tls4moda \
-  tst-tls4modb \
   tst-_res1mod1 \
   tst-_res1mod2 \
-  tst-fini1mod \
-  tst-create1mod \
+  tst-atfork2mod \
   tst-atfork3mod \
   tst-atfork4mod \
-# module-names
+  tst-create1mod \
+  tst-fini1mod \
+  tst-tls4moda \
+  tst-tls4modb \
+  # modules-names
 
 test-modules = $(addprefix $(objpfx),$(addsuffix .so,$(modules-names)))
 
@@ -192,17 +368,30 @@ ifeq ($(build-shared),yes)
 tests: $(test-modules)
 endif
 
+tests-static += \
+  tst-cancel21-static \
+  tst-locale1 \
+  tst-locale2 \
+  # tests-static
 
-tests-static += tst-locale1 tst-locale2 tst-cancel21-static
-
-tests += tst-cancel21-static tst-cond11-static
+tests += \
+  tst-cancel21-static \
+  tst-cond11-static \
+  # tests
 
 # These tests are linked with libc before libpthread
-tests-reverse += tst-cancel5 tst-cancel23 tst-vfork1x tst-vfork2x
+tests-reverse += \
+  tst-cancel5 \
+  tst-cancel23 \
+  tst-vfork1x \
+  tst-vfork2x \
+  # tests-reverse
 
 ifeq ($(run-built-tests),yes)
 ifeq ($(build-shared),yes)
-tests-special += $(objpfx)tst-cleanup0-cmp.out
+tests-special += \
+  $(objpfx)tst-cleanup0-cmp.out \
+  # tests-special
 endif
 endif
 
@@ -286,20 +475,38 @@ $(objpfx)tst-_res1: $(objpfx)tst-_res1mod1.so $(objpfx)tst-_res1mod2.so \
 $(objpfx)tst-pt-tls4: $(shared-thread-library)
 $(objpfx)tst-pt-tls4.out: $(objpfx)tst-tls4moda.so $(objpfx)tst-tls4modb.so
 
-generated += tst-atfork2.mtrace
+generated += \
+  tst-atfork2.mtrace \
+  # generated
 
-generated += $(objpfx)tst-atfork2.mtrace \
-	     $(addsuffix .so,$(strip $(modules-names)))
+generated += \
+  $(addsuffix .so,$(strip $(modules-names))) \
+  $(objpfx)tst-atfork2.mtrace \
+  # generated
 
-tests-internal += tst-cancel25 tst-robust8
+tests-internal += \
+  tst-cancel25 \
+  tst-robust8 \
+  # tests-internal
 
-tests += tst-oncex3 tst-oncex4 tst-oncey3 tst-oncey4
+tests += \
+  tst-oncex3 \
+  tst-oncex4 \
+  tst-oncey3 \
+  tst-oncey4 \
+  # tests
 
-modules-names += tst-join7mod
+modules-names += \
+  tst-join7mod \
+  # modules-names
 
 ifeq ($(build-shared),yes)
-tests-static += tst-cond8-static
-tests += tst-cond8-static
+tests-static += \
+  tst-cond8-static \
+  # tests-static
+tests += \
+  tst-cond8-static \
+  # tests
 endif
 
 CFLAGS-tst-oncex3.c += -fexceptions
diff --git a/sysdeps/pthread/tst-mutex7robust.c b/sysdeps/pthread/tst-robust11.c
similarity index 100%
rename from sysdeps/pthread/tst-mutex7robust.c
rename to sysdeps/pthread/tst-robust11.c
-- 
2.39.2


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

* Re: [PATCH v3 1/2] scripts: Add sort-makefile-lines.py to sort Makefile variables.
  2023-05-10 11:58 ` [PATCH v3 1/2] scripts: Add sort-makefile-lines.py to sort " Carlos O'Donell
@ 2023-05-10 12:42   ` Alejandro Colomar
  2023-05-10 15:07     ` Carlos O'Donell
  2023-05-10 15:49   ` Siddhesh Poyarekar
  1 sibling, 1 reply; 12+ messages in thread
From: Alejandro Colomar @ 2023-05-10 12:42 UTC (permalink / raw)
  To: Carlos O'Donell, libc-alpha, siddhesh


[-- Attachment #1.1: Type: text/plain, Size: 8177 bytes --]

Hi Carlos!


On 5/10/23 13:59, Carlos O'Donell via Libc-alpha wrote:
> v3 sent with stdin/stdout. Thanks for the review! 😄

You're welcome :-)


On 5/10/23 13:58, Carlos O'Donell via Libc-alpha wrote:
> The scripts/sort-makefile-lines.py script sorts Makefile variables
> according to project expected order.
> 
> The script can be used like this:
> 
> $ scripts/sort-makefile-lines.py < elf/Makefile > elf/Makefile.tmp
> $ mv elf/Makefile.tmp elf/Makefile
> ---
> v2->v3: Use stdin/stdout.
> 
>  scripts/sort-makefile-lines.py | 160 +++++++++++++++++++++++++++++++++
>  1 file changed, 160 insertions(+)
>  create mode 100755 scripts/sort-makefile-lines.py
> 
> diff --git a/scripts/sort-makefile-lines.py b/scripts/sort-makefile-lines.py
> new file mode 100755
> index 0000000000..fd657df970
> --- /dev/null
> +++ b/scripts/sort-makefile-lines.py
> @@ -0,0 +1,160 @@
> +#!/usr/bin/python3
> +# Sort Makefile lines as expected by project policy.
> +# Copyright (C) 2023 Free Software Foundation, Inc.
> +# This file is part of the GNU C Library.
> +#
> +# The GNU C Library is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU Lesser General Public
> +# License as published by the Free Software Foundation; either
> +# version 2.1 of the License, or (at your option) any later version.
> +#
> +# The GNU C Library is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +# Lesser General Public License for more details.
> +#
> +# You should have received a copy of the GNU Lesser General Public
> +# License along with the GNU C Library; if not, see
> +# <https://www.gnu.org/licenses/>.
> +
> +# The project consensus is to split Makefile variable assignment
> +# across multiple lines with one value per line.  The values are
> +# then sorted as described below, and terminated with a special
> +# list termination marker.  This splitting makes it much easier
> +# to add new tests to the list since they become just a single
> +# line insertion.  It also makes backports and merges easier
> +# since the new test may not conflict due to the ordering.
> +#
> +# Consensus discussion:
> +# https://inbox.sourceware.org/libc-alpha/f6406204-84f5-adb1-d00e-979ebeebbbde@redhat.com/
> +#
> +# To support cleaning up Makefiles we created this program to
> +# help sort existing lists converted to the new format.
> +#
> +# The program takes as input the Makefile to sort correctly,
> +# and the output file to write the correctly sorted output
> +# (it can be the same file).
> +#
> +# Sorting is only carried out between two special markers:
> +# (a) Marker start is '<variable> += \' (or '= \', or ':= \')
> +# (b) Marker end is '  # <variable>' (whitespace matters)
> +# With everthing between (a) and (b) being sorted accordingly.
> +#
> +# You can use it like this:
> +# $ scripts/sort-makefile-lines.py < elf/Makefile > elf/Makefile.tmp
> +# $ mv elf/Makefile.tmp elf/Makefile
> +#
> +# The Makefile lines in the project are sorted using the
> +# following rules:
> +# - All lines are sorted as-if `LC_COLLATE=C sort`
> +# - Lines that have a numeric suffix and whose leading prefix
> +#   matches exactly are sorted according the numeric suffix
> +#   in increasing numerical order.
> +#
> +# For example:
> +# ~~~
> +# tests += \
> +#   test-a \
> +#   test-b \
> +#   test-b1 \
> +#   test-b2 \
> +#   test-b10 \
> +#   test-b20 \
> +#   test-b100 \
> +#   # tests
> +# ~~~
> +# This example shows tests sorted alphabetically, followed
> +# by a numeric suffix sort in increasing numeric order.
> +#
> +# Cleanups:
> +# - Tests that end in "a" or "b" variants should be renamed to
> +#   end in just the numerical value. For example 'tst-mutex7robust'
> +#   should be renamed to 'tst-mutex12' (the highest numbered test)
> +#   or 'tst-robust11' (the highest numbered test) in order to get
> +#   reasonable ordering.
> +# - Modules that end in "mod" or "mod1" should be renamed. For
> +#   example 'tst-atfork2mod' should be renamed to 'tst-mod-atfork2'
> +#   (test module for atfork2). If there are more than one module
> +#   then they should be named with a suffix that uses [0-9] first
> +#   then [A-Z] next for a total of 36 possible modules per test.
> +#   No manually listed test currently uses more than that (though
> +#   automatically generated tests may; they don't need sorting).
> +# - Avoid including another test and instead refactor into common
> +#   code with all tests including hte common code, then give the
> +#   tests unique names.
> +#
> +# If you have a Makefile that needs converting, then you can
> +# quickly split the values into one-per-line, ensure the start
> +# and end markers are in place, and then run the script to
> +# sort the values.
> +
> +import sys
> +import locale
> +import re
> +import functools
> +
> +def glibc_makefile_numeric(string1, string2):
> +    # Check if string1 has a numeric suffix.
> +    var1 = re.search(r'([0-9]+) \\$', string1)
> +    var2 = re.search(r'([0-9]+) \\$', string2)
> +    if var1 and var2:
> +        if string1[0:var1.span()[0]] == string2[0:var2.span()[0]]:
> +            # string1 and string2 both share a prefix and
> +            # have a numeric suffix that can be compared.
> +            # Sort order is based on the numeric suffix.
> +            return int(var1.group(1)) > int(var2.group(1))
> +    # Default to strcoll.
> +    return locale.strcoll(string1, string2)
> +
> +def sort_lines(lines):
> +
> +    # Use the C locale for language independent collation.
> +    locale.setlocale (locale.LC_ALL, "C")
> +
> +    # Sort using a glibc-specific sorting function.
> +    lines = sorted(lines, key=functools.cmp_to_key(glibc_makefile_numeric))

I believe you're looking for a version sort(1).

$ cat s
bar20x
foo1
bar200
bar19
foo2
bar20
bar2

$ sort -V s
bar2
bar19
bar20
bar20x
bar200
foo1
foo2


I don't know how easy it is to call sort(1) in this script, but it
would probably simplify a big part of it.  And it would also make
unnecessary the renaming of things like 'tst-mutex7robust'.

If using `sort -V` here is complex, maybe it's easier to write a
shell script.

Cheers,
Alex


> +
> +    return lines
> +
> +def sort_makefile_lines():
> +
> +    # Read the whole Makefile.
> +    lines = sys.stdin.readlines()
> +
> +    # Build a list of all start markers (tuple includes name).
> +    startmarks = []
> +    for i in range(len(lines)):
> +        # Look for things like "var = \", "var := \" or "var += \"
> +        # to start the sorted list.
> +        var = re.search(r'^([a-zA-Z0-9-]*) [\+:]?\= \\$', lines[i])
> +        if var:
> +            # Remember the index and the name.
> +            startmarks.append((i, var.group(1)))
> +
> +    # For each start marker try to find a matching end mark
> +    # and build a block that needs sorting.  The end marker
> +    # must have the matching comment name for it to be valid.
> +    rangemarks = []
> +    for sm in startmarks:
> +        # Look for things like "  # var" to end the sorted list.
> +        reg = r'^  # ' + sm[1] + r'$'
> +        for j in range(sm[0] + 1, len(lines)):
> +            if re.search(reg, lines[j]):
> +                # Rembember the block to sort (inclusive).
> +                rangemarks.append((sm[0] + 1, j))
> +                break
> +
> +    # We now have a list of all ranges that need sorting.
> +    # Sort those ranges (inclusive).
> +    for r in rangemarks:
> +        lines[r[0]:r[1]] = sort_lines(lines[r[0]:r[1]])
> +
> +    # Output the whole list with sorted lines to stdout.
> +    [sys.stdout.write(line) for line in lines]
> +
> +
> +def main(argv):
> +    sort_makefile_lines ()
> +
> +if __name__ == '__main__':
> +    main(sys.argv[1:])

-- 
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [PATCH v3 2/2] nptl: Reformat Makefile.
  2023-05-10 11:58 ` [PATCH v3 2/2] nptl: Reformat Makefile Carlos O'Donell
@ 2023-05-10 13:01   ` Siddhesh Poyarekar
  2023-05-10 17:16     ` Carlos O'Donell
  0 siblings, 1 reply; 12+ messages in thread
From: Siddhesh Poyarekar @ 2023-05-10 13:01 UTC (permalink / raw)
  To: Carlos O'Donell, libc-alpha, alx.manpages

On 2023-05-10 07:58, Carlos O'Donell wrote:
> Reflow all long lines adding comment terminators.
> Sort all reflowed text using scripts/sort-makefile-lines.py.
> 
> No code generation changes observed in binary artifacts.
> No regressions on x86_64 and i686.
> ---
>   sysdeps/pthread/Makefile                      | 433 +++++++++++++-----
>   .../{tst-mutex7robust.c => tst-robust11.c}    |   0
>   2 files changed, 320 insertions(+), 113 deletions(-)
>   rename sysdeps/pthread/{tst-mutex7robust.c => tst-robust11.c} (100%)

LGTM.

Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>

> 
> diff --git a/sysdeps/pthread/Makefile b/sysdeps/pthread/Makefile
> index c2f5588bd9..5df1109dd3 100644
> --- a/sysdeps/pthread/Makefile
> +++ b/sysdeps/pthread/Makefile
> @@ -21,9 +21,17 @@ $(objpfx)tst-timer: $(librt)
>   endif
>   
>   ifneq (,$(filter $(subdir),htl nptl))
> -headers += threads.h
> -
> -routines += thrd_current thrd_equal thrd_sleep thrd_yield pthread_atfork
> +headers += \
> +  threads.h \
> +  # headers
> +
> +routines += \
> +  pthread_atfork \
> +  thrd_current \
> +  thrd_equal \
> +  thrd_sleep \
> +  thrd_yield \
> +  # routines
>   
>   $(libpthread-routines-var) += \
>     call_once \
> @@ -48,86 +56,231 @@ $(libpthread-routines-var) += \
>     tss_delete \
>     tss_get \
>     tss_set \
> +  # $(libpthread-routines-var)
>   
> -tests += tst-cnd-basic tst-mtx-trylock tst-cnd-broadcast \
> -	 tst-cnd-timedwait tst-thrd-detach tst-mtx-basic tst-thrd-sleep \
> -	 tst-mtx-recursive tst-tss-basic tst-call-once tst-mtx-timedlock \
> -	 \
> -	 tst-abstime \
> -	 tst-pt-align tst-pt-align3 \
> -	 tst-attr1 \
> -	 tst-backtrace1 \
> -	 tst-bad-schedattr \
> -	 tst-barrier1 tst-barrier2 tst-barrier3 tst-barrier4 \
> -	 tst-basic1 tst-basic2 tst-basic3 tst-basic4 tst-basic5 tst-basic6 \
> -	 tst-basic7 \
> -	 tst-cancel-self tst-cancel-self-cancelstate \
> -	 tst-cancel-self-canceltype tst-cancel-self-testcancel \
> -	 tst-cancel1 tst-cancel2 tst-cancel3 \
> -	 tst-cancel4 tst-cancel5 \
> -	 tst-cancel6 tst-cancel8 tst-cancel9 tst-cancel10 tst-cancel11 \
> -	 tst-cancel12 tst-cancel13 tst-cancel14 tst-cancel15 tst-cancel16 \
> -	 tst-cancel18 tst-cancel19 tst-cancel20 tst-cancel21 \
> -	 tst-cancel22 tst-cancel23 tst-cancel26 tst-cancel27 tst-cancel28 \
> -	 tst-cancel29 \
> -	 tst-cleanup0 tst-cleanup1 tst-cleanup2 tst-cleanup3 \
> -	 tst-clock1 \
> -	 tst-cond-except \
> -	 tst-cond1 tst-cond2 tst-cond3 tst-cond4 tst-cond5 tst-cond6 tst-cond7 \
> -	 tst-cond8 tst-cond9 tst-cond10 tst-cond11 tst-cond12 tst-cond13 \
> -	 tst-cond14 tst-cond15 tst-cond16 tst-cond17 tst-cond18 tst-cond19 \
> -	 tst-cond20 tst-cond21 tst-cond23 tst-cond24 tst-cond25 tst-cond27 \
> -	 tst-create-detached \
> -	 tst-detach1 \
> -	 tst-eintr2 tst-eintr3 tst-eintr4 tst-eintr5 \
> -	 tst-exec1 tst-exec2 tst-exec3 \
> -	 tst-exit1 tst-exit2 tst-exit3 \
> -	 tst-flock1 tst-flock2 \
> -	 tst-fork1 tst-fork2 tst-fork3 tst-fork4 \
> -	 tst-atfork1 \
> -	 tst-getpid3 \
> -	 tst-join1 tst-join2 tst-join3 tst-join4 tst-join5 tst-join6 tst-join7 \
> -	 tst-join8 tst-join9 tst-join10 tst-join11 tst-join12 tst-join13 \
> -	 tst-join14 tst-join15 \
> -	 tst-key1 tst-key2 tst-key3 tst-key4 \
> -	 tst-kill1 tst-kill2 tst-kill3 tst-kill5 tst-kill6 \
> -	 tst-locale1 tst-locale2 \
> -	 tst-memstream \
> -	 tst-mutex-errorcheck tst-mutex1 tst-mutex2 tst-mutex3 tst-mutex4 \
> -	 tst-mutex5 tst-mutex6 tst-mutex7 tst-mutex7robust tst-mutex9 \
> -	 tst-mutex10 tst-mutex11 tst-pthread-mutexattr \
> -	 tst-once1 tst-once2 tst-once3 tst-once4 \
> -	 tst-pt-popen1 \
> -	 tst-raise1 \
> -	 tst-robust1 tst-robust2 tst-robust3 tst-robust4 tst-robust5 \
> -	 tst-robust6 tst-robust7 tst-robust9 tst-robust10 \
> -	 tst-rwlock1 tst-rwlock4 tst-rwlock5 tst-rwlock12 \
> -	 tst-rwlock13 tst-rwlock14 tst-rwlock16 \
> -	 tst-rwlock-tryrdlock-stall tst-rwlock-trywrlock-stall \
> -	 tst-sem1 tst-sem2 tst-sem3 tst-sem4 tst-sem5 tst-sem6 tst-sem7 \
> -	 tst-sem8 tst-sem9 tst-sem10 tst-sem14 tst-sem15 tst-sem16 \
> -	 tst-setuid3 \
> -	 tst-signal1 tst-signal2 \
> -	 tst-signal4 tst-signal5 tst-signal6 tst-signal8 \
> -	 tst-spin1 tst-spin2 tst-spin3 tst-spin4 \
> -	 tst-stack1 \
> -	 tst-stdio1 tst-stdio2 \
> -	 tst-pt-sysconf \
> -	 tst-pt-tls1 tst-pt-tls2 \
> -	 tst-tsd1 tst-tsd2 tst-tsd5 tst-tsd6 \
> -	 tst-umask1 \
> -	 tst-unload \
> -	 tst-unwind-thread \
> -	 tst-pt-vfork1 tst-pt-vfork2 tst-vfork1x tst-vfork2x \
> -	 tst-pthread-exit-signal \
> -	 tst-pthread-setuid-loop \
> -	 tst-pthread_cancel-exited \
> -	 tst-pthread_cancel-select-loop \
> -	 tst-pthread-raise-blocked-self \
> -	 tst-pthread_kill-exited \
> -	 tst-pthread_kill-exiting \
> -	 tst-cancel30 \
> -	 # tests
> +tests += \
> +  tst-abstime \
> +  tst-atfork1 \
> +  tst-attr1 \
> +  tst-backtrace1 \
> +  tst-bad-schedattr \
> +  tst-barrier1 \
> +  tst-barrier2 \
> +  tst-barrier3 \
> +  tst-barrier4 \
> +  tst-basic1 \
> +  tst-basic2 \
> +  tst-basic3 \
> +  tst-basic4 \
> +  tst-basic5 \
> +  tst-basic6 \
> +  tst-basic7 \
> +  tst-call-once \
> +  tst-cancel-self \
> +  tst-cancel-self-cancelstate \
> +  tst-cancel-self-canceltype \
> +  tst-cancel-self-testcancel \
> +  tst-cancel1 \
> +  tst-cancel2 \
> +  tst-cancel3 \
> +  tst-cancel4 \
> +  tst-cancel5 \
> +  tst-cancel6 \
> +  tst-cancel8 \
> +  tst-cancel9 \
> +  tst-cancel10 \
> +  tst-cancel11 \
> +  tst-cancel12 \
> +  tst-cancel13 \
> +  tst-cancel14 \
> +  tst-cancel15 \
> +  tst-cancel16 \
> +  tst-cancel18 \
> +  tst-cancel19 \
> +  tst-cancel20 \
> +  tst-cancel21 \
> +  tst-cancel22 \
> +  tst-cancel23 \
> +  tst-cancel26 \
> +  tst-cancel27 \
> +  tst-cancel28 \
> +  tst-cancel29 \
> +  tst-cancel30 \
> +  tst-cleanup0 \
> +  tst-cleanup1 \
> +  tst-cleanup2 \
> +  tst-cleanup3 \
> +  tst-clock1 \
> +  tst-cnd-basic \
> +  tst-cnd-broadcast \
> +  tst-cnd-timedwait \
> +  tst-cond-except \
> +  tst-cond1 \
> +  tst-cond2 \
> +  tst-cond3 \
> +  tst-cond4 \
> +  tst-cond5 \
> +  tst-cond6 \
> +  tst-cond7 \
> +  tst-cond8 \
> +  tst-cond9 \
> +  tst-cond10 \
> +  tst-cond11 \
> +  tst-cond12 \
> +  tst-cond13 \
> +  tst-cond14 \
> +  tst-cond15 \
> +  tst-cond16 \
> +  tst-cond17 \
> +  tst-cond18 \
> +  tst-cond19 \
> +  tst-cond20 \
> +  tst-cond21 \
> +  tst-cond23 \
> +  tst-cond24 \
> +  tst-cond25 \
> +  tst-cond27 \
> +  tst-create-detached \
> +  tst-detach1 \
> +  tst-eintr2 \
> +  tst-eintr3 \
> +  tst-eintr4 \
> +  tst-eintr5 \
> +  tst-exec1 \
> +  tst-exec2 \
> +  tst-exec3 \
> +  tst-exit1 \
> +  tst-exit2 \
> +  tst-exit3 \
> +  tst-flock1 \
> +  tst-flock2 \
> +  tst-fork1 \
> +  tst-fork2 \
> +  tst-fork3 \
> +  tst-fork4 \
> +  tst-getpid3 \
> +  tst-join1 \
> +  tst-join2 \
> +  tst-join3 \
> +  tst-join4 \
> +  tst-join5 \
> +  tst-join6 \
> +  tst-join7 \
> +  tst-join8 \
> +  tst-join9 \
> +  tst-join10 \
> +  tst-join11 \
> +  tst-join12 \
> +  tst-join13 \
> +  tst-join14 \
> +  tst-join15 \
> +  tst-key1 \
> +  tst-key2 \
> +  tst-key3 \
> +  tst-key4 \
> +  tst-kill1 \
> +  tst-kill2 \
> +  tst-kill3 \
> +  tst-kill5 \
> +  tst-kill6 \
> +  tst-locale1 \
> +  tst-locale2 \
> +  tst-memstream \
> +  tst-mtx-basic \
> +  tst-mtx-recursive \
> +  tst-mtx-timedlock \
> +  tst-mtx-trylock \
> +  tst-mutex-errorcheck \
> +  tst-mutex1 \
> +  tst-mutex2 \
> +  tst-mutex3 \
> +  tst-mutex4 \
> +  tst-mutex5 \
> +  tst-mutex6 \
> +  tst-mutex7 \
> +  tst-mutex9 \
> +  tst-mutex10 \
> +  tst-mutex11 \
> +  tst-once1 \
> +  tst-once2 \
> +  tst-once3 \
> +  tst-once4 \
> +  tst-pt-align \
> +  tst-pt-align3 \
> +  tst-pt-popen1 \
> +  tst-pt-sysconf \
> +  tst-pt-tls1 \
> +  tst-pt-tls2 \
> +  tst-pt-vfork1 \
> +  tst-pt-vfork2 \
> +  tst-pthread-exit-signal \
> +  tst-pthread-mutexattr \
> +  tst-pthread-raise-blocked-self \
> +  tst-pthread-setuid-loop \
> +  tst-pthread_cancel-exited \
> +  tst-pthread_cancel-select-loop \
> +  tst-pthread_kill-exited \
> +  tst-pthread_kill-exiting \
> +  tst-raise1 \
> +  tst-robust1 \
> +  tst-robust2 \
> +  tst-robust3 \
> +  tst-robust4 \
> +  tst-robust5 \
> +  tst-robust6 \
> +  tst-robust7 \
> +  tst-robust9 \
> +  tst-robust10 \
> +  tst-robust11 \
> +  tst-rwlock-tryrdlock-stall \
> +  tst-rwlock-trywrlock-stall \
> +  tst-rwlock1 \
> +  tst-rwlock4 \
> +  tst-rwlock5 \
> +  tst-rwlock12 \
> +  tst-rwlock13 \
> +  tst-rwlock14 \
> +  tst-rwlock16 \
> +  tst-sem1 \
> +  tst-sem2 \
> +  tst-sem3 \
> +  tst-sem4 \
> +  tst-sem5 \
> +  tst-sem6 \
> +  tst-sem7 \
> +  tst-sem8 \
> +  tst-sem9 \
> +  tst-sem10 \
> +  tst-sem14 \
> +  tst-sem15 \
> +  tst-sem16 \
> +  tst-setuid3 \
> +  tst-signal1 \
> +  tst-signal2 \
> +  tst-signal4 \
> +  tst-signal5 \
> +  tst-signal6 \
> +  tst-signal8 \
> +  tst-spin1 \
> +  tst-spin2 \
> +  tst-spin3 \
> +  tst-spin4 \
> +  tst-stack1 \
> +  tst-stdio1 \
> +  tst-stdio2 \
> +  tst-thrd-detach \
> +  tst-thrd-sleep \
> +  tst-tsd1 \
> +  tst-tsd2 \
> +  tst-tsd5 \
> +  tst-tsd6 \
> +  tst-tss-basic \
> +  tst-umask1 \
> +  tst-unload \
> +  tst-unwind-thread \
> +  tst-vfork1x \
> +  tst-vfork2x \
> +  # tests
>   
>   tests-time64 += \
>     tst-abstime-time64 \
> @@ -138,47 +291,70 @@ tests-time64 += \
>     tst-rwlock14-time64 \
>     tst-sem5-time64 \
>     tst-thrd-sleep-time64 \
> +  # tests-time64
>   
>   static-only-routines = pthread_atfork
>   
>   # Files which must not be linked with libpthread.
> -tests-nolibpthread += tst-unload
> +tests-nolibpthread += \
> +  tst-unload \
> +  # tests-nolibpthread
>   
>   # GCC-4.9 compiles 'sprintf(NULL, ...)' into UD2 on x86_64 without -fno-builtin
>   CFLAGS-tst-cleanup2.c += -fno-builtin
>   CFLAGS-tst-cleanupx2.c += -fno-builtin
>   
> -tests += tst-cancelx2 tst-cancelx3 tst-cancelx6 tst-cancelx8 tst-cancelx9 \
> -	 tst-cancelx4 tst-cancelx5 \
> -	 tst-cancelx10 tst-cancelx11 tst-cancelx12 tst-cancelx13 tst-cancelx14 \
> -	 tst-cancelx15 tst-cancelx16 tst-cancelx18 tst-cancelx20 tst-cancelx21 \
> -	 tst-cleanupx0 tst-cleanupx1 tst-cleanupx2 tst-cleanupx3
> +tests += \
> +  tst-cancelx2 \
> +  tst-cancelx3 \
> +  tst-cancelx4 \
> +  tst-cancelx5 \
> +  tst-cancelx6 \
> +  tst-cancelx8 \
> +  tst-cancelx9 \
> +  tst-cancelx10 \
> +  tst-cancelx11 \
> +  tst-cancelx12 \
> +  tst-cancelx13 \
> +  tst-cancelx14 \
> +  tst-cancelx15 \
> +  tst-cancelx16 \
> +  tst-cancelx18 \
> +  tst-cancelx20 \
> +  tst-cancelx21 \
> +  tst-cleanupx0 \
> +  tst-cleanupx1 \
> +  tst-cleanupx2 \
> +  tst-cleanupx3 \
> +  # tests
>   
>   ifeq ($(build-shared),yes)
>   tests += \
> -  tst-atfork2 \
> -  tst-pt-tls4 \
>     tst-_res1 \
> -  tst-fini1 \
> -  tst-create1 \
> +  tst-atfork2 \
>     tst-atfork3 \
>     tst-atfork4 \
> -# tests
> +  tst-create1 \
> +  tst-fini1 \
> +  tst-pt-tls4 \
> +  # tests
>   
> -tests-nolibpthread += tst-fini1
> +tests-nolibpthread += \
> +  tst-fini1 \
> +  # tests-nolibpthread
>   endif
>   
>   modules-names += \
> -  tst-atfork2mod \
> -  tst-tls4moda \
> -  tst-tls4modb \
>     tst-_res1mod1 \
>     tst-_res1mod2 \
> -  tst-fini1mod \
> -  tst-create1mod \
> +  tst-atfork2mod \
>     tst-atfork3mod \
>     tst-atfork4mod \
> -# module-names
> +  tst-create1mod \
> +  tst-fini1mod \
> +  tst-tls4moda \
> +  tst-tls4modb \
> +  # modules-names
>   
>   test-modules = $(addprefix $(objpfx),$(addsuffix .so,$(modules-names)))
>   
> @@ -192,17 +368,30 @@ ifeq ($(build-shared),yes)
>   tests: $(test-modules)
>   endif
>   
> +tests-static += \
> +  tst-cancel21-static \
> +  tst-locale1 \
> +  tst-locale2 \
> +  # tests-static
>   
> -tests-static += tst-locale1 tst-locale2 tst-cancel21-static
> -
> -tests += tst-cancel21-static tst-cond11-static
> +tests += \
> +  tst-cancel21-static \
> +  tst-cond11-static \
> +  # tests
>   
>   # These tests are linked with libc before libpthread
> -tests-reverse += tst-cancel5 tst-cancel23 tst-vfork1x tst-vfork2x
> +tests-reverse += \
> +  tst-cancel5 \
> +  tst-cancel23 \
> +  tst-vfork1x \
> +  tst-vfork2x \
> +  # tests-reverse
>   
>   ifeq ($(run-built-tests),yes)
>   ifeq ($(build-shared),yes)
> -tests-special += $(objpfx)tst-cleanup0-cmp.out
> +tests-special += \
> +  $(objpfx)tst-cleanup0-cmp.out \
> +  # tests-special
>   endif
>   endif
>   
> @@ -286,20 +475,38 @@ $(objpfx)tst-_res1: $(objpfx)tst-_res1mod1.so $(objpfx)tst-_res1mod2.so \
>   $(objpfx)tst-pt-tls4: $(shared-thread-library)
>   $(objpfx)tst-pt-tls4.out: $(objpfx)tst-tls4moda.so $(objpfx)tst-tls4modb.so
>   
> -generated += tst-atfork2.mtrace
> +generated += \
> +  tst-atfork2.mtrace \
> +  # generated
>   
> -generated += $(objpfx)tst-atfork2.mtrace \
> -	     $(addsuffix .so,$(strip $(modules-names)))
> +generated += \
> +  $(addsuffix .so,$(strip $(modules-names))) \
> +  $(objpfx)tst-atfork2.mtrace \
> +  # generated
>   
> -tests-internal += tst-cancel25 tst-robust8
> +tests-internal += \
> +  tst-cancel25 \
> +  tst-robust8 \
> +  # tests-internal
>   
> -tests += tst-oncex3 tst-oncex4 tst-oncey3 tst-oncey4
> +tests += \
> +  tst-oncex3 \
> +  tst-oncex4 \
> +  tst-oncey3 \
> +  tst-oncey4 \
> +  # tests
>   
> -modules-names += tst-join7mod
> +modules-names += \
> +  tst-join7mod \
> +  # modules-names
>   
>   ifeq ($(build-shared),yes)
> -tests-static += tst-cond8-static
> -tests += tst-cond8-static
> +tests-static += \
> +  tst-cond8-static \
> +  # tests-static
> +tests += \
> +  tst-cond8-static \
> +  # tests
>   endif
>   
>   CFLAGS-tst-oncex3.c += -fexceptions
> diff --git a/sysdeps/pthread/tst-mutex7robust.c b/sysdeps/pthread/tst-robust11.c
> similarity index 100%
> rename from sysdeps/pthread/tst-mutex7robust.c
> rename to sysdeps/pthread/tst-robust11.c

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

* Re: [PATCH v3 1/2] scripts: Add sort-makefile-lines.py to sort Makefile variables.
  2023-05-10 12:42   ` Alejandro Colomar
@ 2023-05-10 15:07     ` Carlos O'Donell
  2023-05-10 15:12       ` Alejandro Colomar
  0 siblings, 1 reply; 12+ messages in thread
From: Carlos O'Donell @ 2023-05-10 15:07 UTC (permalink / raw)
  To: Alejandro Colomar, libc-alpha, siddhesh

On 5/10/23 08:42, Alejandro Colomar wrote:
> I believe you're looking for a version sort(1).

Yes, sort -V does work, the downside is the integration to handle the blocks
of text in the Makefile.

> I don't know how easy it is to call sort(1) in this script, but it
> would probably simplify a big part of it.  And it would also make
> unnecessary the renaming of things like 'tst-mutex7robust'.

You don't *need* to rename any tests in the new version of the code, I took
out the error case and just sort as the new function sorts (like sort -V does).

> If using `sort -V` here is complex, maybe it's easier to write a
> shell script.

I would want to use bash with arrays to handle lines and processing the start
and end blocks. It could be possible to do it in an equivalent number of lines,
but I think the standalone python is easier to read and maintain (less shell
quoting issues with lines of text).

Any objection to the proposed python?

-- 
Cheers,
Carlos.


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

* Re: [PATCH v3 1/2] scripts: Add sort-makefile-lines.py to sort Makefile variables.
  2023-05-10 15:07     ` Carlos O'Donell
@ 2023-05-10 15:12       ` Alejandro Colomar
  2023-05-10 17:30         ` Carlos O'Donell
  0 siblings, 1 reply; 12+ messages in thread
From: Alejandro Colomar @ 2023-05-10 15:12 UTC (permalink / raw)
  To: Carlos O'Donell, libc-alpha, siddhesh


[-- Attachment #1.1: Type: text/plain, Size: 1657 bytes --]

Hi Carlos,

On 5/10/23 17:07, Carlos O'Donell wrote:
> On 5/10/23 08:42, Alejandro Colomar wrote:
>> I believe you're looking for a version sort(1).
> 
> Yes, sort -V does work, the downside is the integration to handle the blocks
> of text in the Makefile.

Yup, I've been thinking about it, and a pipeline seems non-obvious.
It would need some bash magic, or maybe some non-trivial awk(1) or perl(1).

> 
>> I don't know how easy it is to call sort(1) in this script, but it
>> would probably simplify a big part of it.  And it would also make
>> unnecessary the renaming of things like 'tst-mutex7robust'.
> 
> You don't *need* to rename any tests in the new version of the code, I took
> out the error case and just sort as the new function sorts (like sort -V does).
> 
>> If using `sort -V` here is complex, maybe it's easier to write a
>> shell script.
> 
> I would want to use bash with arrays to handle lines and processing the start
> and end blocks. It could be possible to do it in an equivalent number of lines,
> but I think the standalone python is easier to read and maintain (less shell
> quoting issues with lines of text).
> 
> Any objection to the proposed python?

Not really.  I was just suggesting it case it didn't occur to you.
Maybe a small objection would be in the naming of the functions.
If it sorts as if `sort -V`, how about calling it something like
version_sort?  And maybe a comment stating it behaves as if `sort -V`?

Other than that, I'm fine with it :)

Cheers,
Alex

> 

-- 
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [PATCH v3 1/2] scripts: Add sort-makefile-lines.py to sort Makefile variables.
  2023-05-10 11:58 ` [PATCH v3 1/2] scripts: Add sort-makefile-lines.py to sort " Carlos O'Donell
  2023-05-10 12:42   ` Alejandro Colomar
@ 2023-05-10 15:49   ` Siddhesh Poyarekar
  2023-05-10 17:17     ` Carlos O'Donell
  1 sibling, 1 reply; 12+ messages in thread
From: Siddhesh Poyarekar @ 2023-05-10 15:49 UTC (permalink / raw)
  To: Carlos O'Donell, libc-alpha, alx.manpages

On 2023-05-10 07:58, Carlos O'Donell wrote:
> The scripts/sort-makefile-lines.py script sorts Makefile variables
> according to project expected order.
> 
> The script can be used like this:
> 
> $ scripts/sort-makefile-lines.py < elf/Makefile > elf/Makefile.tmp
> $ mv elf/Makefile.tmp elf/Makefile
> ---
> v2->v3: Use stdin/stdout.
> 
>   scripts/sort-makefile-lines.py | 160 +++++++++++++++++++++++++++++++++
>   1 file changed, 160 insertions(+)
>   create mode 100755 scripts/sort-makefile-lines.py

LGTM.

Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>

> 
> diff --git a/scripts/sort-makefile-lines.py b/scripts/sort-makefile-lines.py
> new file mode 100755
> index 0000000000..fd657df970
> --- /dev/null
> +++ b/scripts/sort-makefile-lines.py
> @@ -0,0 +1,160 @@
> +#!/usr/bin/python3
> +# Sort Makefile lines as expected by project policy.
> +# Copyright (C) 2023 Free Software Foundation, Inc.
> +# This file is part of the GNU C Library.
> +#
> +# The GNU C Library is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU Lesser General Public
> +# License as published by the Free Software Foundation; either
> +# version 2.1 of the License, or (at your option) any later version.
> +#
> +# The GNU C Library is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +# Lesser General Public License for more details.
> +#
> +# You should have received a copy of the GNU Lesser General Public
> +# License along with the GNU C Library; if not, see
> +# <https://www.gnu.org/licenses/>.
> +
> +# The project consensus is to split Makefile variable assignment
> +# across multiple lines with one value per line.  The values are
> +# then sorted as described below, and terminated with a special
> +# list termination marker.  This splitting makes it much easier
> +# to add new tests to the list since they become just a single
> +# line insertion.  It also makes backports and merges easier
> +# since the new test may not conflict due to the ordering.
> +#
> +# Consensus discussion:
> +# https://inbox.sourceware.org/libc-alpha/f6406204-84f5-adb1-d00e-979ebeebbbde@redhat.com/
> +#
> +# To support cleaning up Makefiles we created this program to
> +# help sort existing lists converted to the new format.
> +#
> +# The program takes as input the Makefile to sort correctly,
> +# and the output file to write the correctly sorted output
> +# (it can be the same file).
> +#
> +# Sorting is only carried out between two special markers:
> +# (a) Marker start is '<variable> += \' (or '= \', or ':= \')
> +# (b) Marker end is '  # <variable>' (whitespace matters)
> +# With everthing between (a) and (b) being sorted accordingly.
> +#
> +# You can use it like this:
> +# $ scripts/sort-makefile-lines.py < elf/Makefile > elf/Makefile.tmp
> +# $ mv elf/Makefile.tmp elf/Makefile
> +#
> +# The Makefile lines in the project are sorted using the
> +# following rules:
> +# - All lines are sorted as-if `LC_COLLATE=C sort`
> +# - Lines that have a numeric suffix and whose leading prefix
> +#   matches exactly are sorted according the numeric suffix
> +#   in increasing numerical order.
> +#
> +# For example:
> +# ~~~
> +# tests += \
> +#   test-a \
> +#   test-b \
> +#   test-b1 \
> +#   test-b2 \
> +#   test-b10 \
> +#   test-b20 \
> +#   test-b100 \
> +#   # tests
> +# ~~~
> +# This example shows tests sorted alphabetically, followed
> +# by a numeric suffix sort in increasing numeric order.
> +#
> +# Cleanups:
> +# - Tests that end in "a" or "b" variants should be renamed to
> +#   end in just the numerical value. For example 'tst-mutex7robust'
> +#   should be renamed to 'tst-mutex12' (the highest numbered test)
> +#   or 'tst-robust11' (the highest numbered test) in order to get
> +#   reasonable ordering.
> +# - Modules that end in "mod" or "mod1" should be renamed. For
> +#   example 'tst-atfork2mod' should be renamed to 'tst-mod-atfork2'
> +#   (test module for atfork2). If there are more than one module
> +#   then they should be named with a suffix that uses [0-9] first
> +#   then [A-Z] next for a total of 36 possible modules per test.
> +#   No manually listed test currently uses more than that (though
> +#   automatically generated tests may; they don't need sorting).
> +# - Avoid including another test and instead refactor into common
> +#   code with all tests including hte common code, then give the
> +#   tests unique names.
> +#
> +# If you have a Makefile that needs converting, then you can
> +# quickly split the values into one-per-line, ensure the start
> +# and end markers are in place, and then run the script to
> +# sort the values.
> +
> +import sys
> +import locale
> +import re
> +import functools
> +
> +def glibc_makefile_numeric(string1, string2):
> +    # Check if string1 has a numeric suffix.
> +    var1 = re.search(r'([0-9]+) \\$', string1)
> +    var2 = re.search(r'([0-9]+) \\$', string2)
> +    if var1 and var2:
> +        if string1[0:var1.span()[0]] == string2[0:var2.span()[0]]:
> +            # string1 and string2 both share a prefix and
> +            # have a numeric suffix that can be compared.
> +            # Sort order is based on the numeric suffix.
> +            return int(var1.group(1)) > int(var2.group(1))
> +    # Default to strcoll.
> +    return locale.strcoll(string1, string2)
> +
> +def sort_lines(lines):
> +
> +    # Use the C locale for language independent collation.
> +    locale.setlocale (locale.LC_ALL, "C")
> +
> +    # Sort using a glibc-specific sorting function.
> +    lines = sorted(lines, key=functools.cmp_to_key(glibc_makefile_numeric))
> +
> +    return lines
> +
> +def sort_makefile_lines():
> +
> +    # Read the whole Makefile.
> +    lines = sys.stdin.readlines()
> +
> +    # Build a list of all start markers (tuple includes name).
> +    startmarks = []
> +    for i in range(len(lines)):
> +        # Look for things like "var = \", "var := \" or "var += \"
> +        # to start the sorted list.
> +        var = re.search(r'^([a-zA-Z0-9-]*) [\+:]?\= \\$', lines[i])
> +        if var:
> +            # Remember the index and the name.
> +            startmarks.append((i, var.group(1)))
> +
> +    # For each start marker try to find a matching end mark
> +    # and build a block that needs sorting.  The end marker
> +    # must have the matching comment name for it to be valid.
> +    rangemarks = []
> +    for sm in startmarks:
> +        # Look for things like "  # var" to end the sorted list.
> +        reg = r'^  # ' + sm[1] + r'$'
> +        for j in range(sm[0] + 1, len(lines)):
> +            if re.search(reg, lines[j]):
> +                # Rembember the block to sort (inclusive).
> +                rangemarks.append((sm[0] + 1, j))
> +                break
> +
> +    # We now have a list of all ranges that need sorting.
> +    # Sort those ranges (inclusive).
> +    for r in rangemarks:
> +        lines[r[0]:r[1]] = sort_lines(lines[r[0]:r[1]])
> +
> +    # Output the whole list with sorted lines to stdout.
> +    [sys.stdout.write(line) for line in lines]
> +
> +
> +def main(argv):
> +    sort_makefile_lines ()
> +
> +if __name__ == '__main__':
> +    main(sys.argv[1:])

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

* Re: [PATCH v3 2/2] nptl: Reformat Makefile.
  2023-05-10 13:01   ` Siddhesh Poyarekar
@ 2023-05-10 17:16     ` Carlos O'Donell
  0 siblings, 0 replies; 12+ messages in thread
From: Carlos O'Donell @ 2023-05-10 17:16 UTC (permalink / raw)
  To: Siddhesh Poyarekar, libc-alpha, alx.manpages

On 5/10/23 09:01, Siddhesh Poyarekar wrote:
> On 2023-05-10 07:58, Carlos O'Donell wrote:
>> Reflow all long lines adding comment terminators.
>> Sort all reflowed text using scripts/sort-makefile-lines.py.
>>
>> No code generation changes observed in binary artifacts.
>> No regressions on x86_64 and i686.
>> ---
>>   sysdeps/pthread/Makefile                      | 433 +++++++++++++-----
>>   .../{tst-mutex7robust.c => tst-robust11.c}    |   0
>>   2 files changed, 320 insertions(+), 113 deletions(-)
>>   rename sysdeps/pthread/{tst-mutex7robust.c => tst-robust11.c} (100%)
> 
> LGTM.
> 
> Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>

Thanks! Pushed.
 
>>
>> diff --git a/sysdeps/pthread/Makefile b/sysdeps/pthread/Makefile
>> index c2f5588bd9..5df1109dd3 100644
>> --- a/sysdeps/pthread/Makefile
>> +++ b/sysdeps/pthread/Makefile
>> @@ -21,9 +21,17 @@ $(objpfx)tst-timer: $(librt)
>>   endif
>>     ifneq (,$(filter $(subdir),htl nptl))
>> -headers += threads.h
>> -
>> -routines += thrd_current thrd_equal thrd_sleep thrd_yield pthread_atfork
>> +headers += \
>> +  threads.h \
>> +  # headers
>> +
>> +routines += \
>> +  pthread_atfork \
>> +  thrd_current \
>> +  thrd_equal \
>> +  thrd_sleep \
>> +  thrd_yield \
>> +  # routines
>>     $(libpthread-routines-var) += \
>>     call_once \
>> @@ -48,86 +56,231 @@ $(libpthread-routines-var) += \
>>     tss_delete \
>>     tss_get \
>>     tss_set \
>> +  # $(libpthread-routines-var)
>>   -tests += tst-cnd-basic tst-mtx-trylock tst-cnd-broadcast \
>> -     tst-cnd-timedwait tst-thrd-detach tst-mtx-basic tst-thrd-sleep \
>> -     tst-mtx-recursive tst-tss-basic tst-call-once tst-mtx-timedlock \
>> -     \
>> -     tst-abstime \
>> -     tst-pt-align tst-pt-align3 \
>> -     tst-attr1 \
>> -     tst-backtrace1 \
>> -     tst-bad-schedattr \
>> -     tst-barrier1 tst-barrier2 tst-barrier3 tst-barrier4 \
>> -     tst-basic1 tst-basic2 tst-basic3 tst-basic4 tst-basic5 tst-basic6 \
>> -     tst-basic7 \
>> -     tst-cancel-self tst-cancel-self-cancelstate \
>> -     tst-cancel-self-canceltype tst-cancel-self-testcancel \
>> -     tst-cancel1 tst-cancel2 tst-cancel3 \
>> -     tst-cancel4 tst-cancel5 \
>> -     tst-cancel6 tst-cancel8 tst-cancel9 tst-cancel10 tst-cancel11 \
>> -     tst-cancel12 tst-cancel13 tst-cancel14 tst-cancel15 tst-cancel16 \
>> -     tst-cancel18 tst-cancel19 tst-cancel20 tst-cancel21 \
>> -     tst-cancel22 tst-cancel23 tst-cancel26 tst-cancel27 tst-cancel28 \
>> -     tst-cancel29 \
>> -     tst-cleanup0 tst-cleanup1 tst-cleanup2 tst-cleanup3 \
>> -     tst-clock1 \
>> -     tst-cond-except \
>> -     tst-cond1 tst-cond2 tst-cond3 tst-cond4 tst-cond5 tst-cond6 tst-cond7 \
>> -     tst-cond8 tst-cond9 tst-cond10 tst-cond11 tst-cond12 tst-cond13 \
>> -     tst-cond14 tst-cond15 tst-cond16 tst-cond17 tst-cond18 tst-cond19 \
>> -     tst-cond20 tst-cond21 tst-cond23 tst-cond24 tst-cond25 tst-cond27 \
>> -     tst-create-detached \
>> -     tst-detach1 \
>> -     tst-eintr2 tst-eintr3 tst-eintr4 tst-eintr5 \
>> -     tst-exec1 tst-exec2 tst-exec3 \
>> -     tst-exit1 tst-exit2 tst-exit3 \
>> -     tst-flock1 tst-flock2 \
>> -     tst-fork1 tst-fork2 tst-fork3 tst-fork4 \
>> -     tst-atfork1 \
>> -     tst-getpid3 \
>> -     tst-join1 tst-join2 tst-join3 tst-join4 tst-join5 tst-join6 tst-join7 \
>> -     tst-join8 tst-join9 tst-join10 tst-join11 tst-join12 tst-join13 \
>> -     tst-join14 tst-join15 \
>> -     tst-key1 tst-key2 tst-key3 tst-key4 \
>> -     tst-kill1 tst-kill2 tst-kill3 tst-kill5 tst-kill6 \
>> -     tst-locale1 tst-locale2 \
>> -     tst-memstream \
>> -     tst-mutex-errorcheck tst-mutex1 tst-mutex2 tst-mutex3 tst-mutex4 \
>> -     tst-mutex5 tst-mutex6 tst-mutex7 tst-mutex7robust tst-mutex9 \
>> -     tst-mutex10 tst-mutex11 tst-pthread-mutexattr \
>> -     tst-once1 tst-once2 tst-once3 tst-once4 \
>> -     tst-pt-popen1 \
>> -     tst-raise1 \
>> -     tst-robust1 tst-robust2 tst-robust3 tst-robust4 tst-robust5 \
>> -     tst-robust6 tst-robust7 tst-robust9 tst-robust10 \
>> -     tst-rwlock1 tst-rwlock4 tst-rwlock5 tst-rwlock12 \
>> -     tst-rwlock13 tst-rwlock14 tst-rwlock16 \
>> -     tst-rwlock-tryrdlock-stall tst-rwlock-trywrlock-stall \
>> -     tst-sem1 tst-sem2 tst-sem3 tst-sem4 tst-sem5 tst-sem6 tst-sem7 \
>> -     tst-sem8 tst-sem9 tst-sem10 tst-sem14 tst-sem15 tst-sem16 \
>> -     tst-setuid3 \
>> -     tst-signal1 tst-signal2 \
>> -     tst-signal4 tst-signal5 tst-signal6 tst-signal8 \
>> -     tst-spin1 tst-spin2 tst-spin3 tst-spin4 \
>> -     tst-stack1 \
>> -     tst-stdio1 tst-stdio2 \
>> -     tst-pt-sysconf \
>> -     tst-pt-tls1 tst-pt-tls2 \
>> -     tst-tsd1 tst-tsd2 tst-tsd5 tst-tsd6 \
>> -     tst-umask1 \
>> -     tst-unload \
>> -     tst-unwind-thread \
>> -     tst-pt-vfork1 tst-pt-vfork2 tst-vfork1x tst-vfork2x \
>> -     tst-pthread-exit-signal \
>> -     tst-pthread-setuid-loop \
>> -     tst-pthread_cancel-exited \
>> -     tst-pthread_cancel-select-loop \
>> -     tst-pthread-raise-blocked-self \
>> -     tst-pthread_kill-exited \
>> -     tst-pthread_kill-exiting \
>> -     tst-cancel30 \
>> -     # tests
>> +tests += \
>> +  tst-abstime \
>> +  tst-atfork1 \
>> +  tst-attr1 \
>> +  tst-backtrace1 \
>> +  tst-bad-schedattr \
>> +  tst-barrier1 \
>> +  tst-barrier2 \
>> +  tst-barrier3 \
>> +  tst-barrier4 \
>> +  tst-basic1 \
>> +  tst-basic2 \
>> +  tst-basic3 \
>> +  tst-basic4 \
>> +  tst-basic5 \
>> +  tst-basic6 \
>> +  tst-basic7 \
>> +  tst-call-once \
>> +  tst-cancel-self \
>> +  tst-cancel-self-cancelstate \
>> +  tst-cancel-self-canceltype \
>> +  tst-cancel-self-testcancel \
>> +  tst-cancel1 \
>> +  tst-cancel2 \
>> +  tst-cancel3 \
>> +  tst-cancel4 \
>> +  tst-cancel5 \
>> +  tst-cancel6 \
>> +  tst-cancel8 \
>> +  tst-cancel9 \
>> +  tst-cancel10 \
>> +  tst-cancel11 \
>> +  tst-cancel12 \
>> +  tst-cancel13 \
>> +  tst-cancel14 \
>> +  tst-cancel15 \
>> +  tst-cancel16 \
>> +  tst-cancel18 \
>> +  tst-cancel19 \
>> +  tst-cancel20 \
>> +  tst-cancel21 \
>> +  tst-cancel22 \
>> +  tst-cancel23 \
>> +  tst-cancel26 \
>> +  tst-cancel27 \
>> +  tst-cancel28 \
>> +  tst-cancel29 \
>> +  tst-cancel30 \
>> +  tst-cleanup0 \
>> +  tst-cleanup1 \
>> +  tst-cleanup2 \
>> +  tst-cleanup3 \
>> +  tst-clock1 \
>> +  tst-cnd-basic \
>> +  tst-cnd-broadcast \
>> +  tst-cnd-timedwait \
>> +  tst-cond-except \
>> +  tst-cond1 \
>> +  tst-cond2 \
>> +  tst-cond3 \
>> +  tst-cond4 \
>> +  tst-cond5 \
>> +  tst-cond6 \
>> +  tst-cond7 \
>> +  tst-cond8 \
>> +  tst-cond9 \
>> +  tst-cond10 \
>> +  tst-cond11 \
>> +  tst-cond12 \
>> +  tst-cond13 \
>> +  tst-cond14 \
>> +  tst-cond15 \
>> +  tst-cond16 \
>> +  tst-cond17 \
>> +  tst-cond18 \
>> +  tst-cond19 \
>> +  tst-cond20 \
>> +  tst-cond21 \
>> +  tst-cond23 \
>> +  tst-cond24 \
>> +  tst-cond25 \
>> +  tst-cond27 \
>> +  tst-create-detached \
>> +  tst-detach1 \
>> +  tst-eintr2 \
>> +  tst-eintr3 \
>> +  tst-eintr4 \
>> +  tst-eintr5 \
>> +  tst-exec1 \
>> +  tst-exec2 \
>> +  tst-exec3 \
>> +  tst-exit1 \
>> +  tst-exit2 \
>> +  tst-exit3 \
>> +  tst-flock1 \
>> +  tst-flock2 \
>> +  tst-fork1 \
>> +  tst-fork2 \
>> +  tst-fork3 \
>> +  tst-fork4 \
>> +  tst-getpid3 \
>> +  tst-join1 \
>> +  tst-join2 \
>> +  tst-join3 \
>> +  tst-join4 \
>> +  tst-join5 \
>> +  tst-join6 \
>> +  tst-join7 \
>> +  tst-join8 \
>> +  tst-join9 \
>> +  tst-join10 \
>> +  tst-join11 \
>> +  tst-join12 \
>> +  tst-join13 \
>> +  tst-join14 \
>> +  tst-join15 \
>> +  tst-key1 \
>> +  tst-key2 \
>> +  tst-key3 \
>> +  tst-key4 \
>> +  tst-kill1 \
>> +  tst-kill2 \
>> +  tst-kill3 \
>> +  tst-kill5 \
>> +  tst-kill6 \
>> +  tst-locale1 \
>> +  tst-locale2 \
>> +  tst-memstream \
>> +  tst-mtx-basic \
>> +  tst-mtx-recursive \
>> +  tst-mtx-timedlock \
>> +  tst-mtx-trylock \
>> +  tst-mutex-errorcheck \
>> +  tst-mutex1 \
>> +  tst-mutex2 \
>> +  tst-mutex3 \
>> +  tst-mutex4 \
>> +  tst-mutex5 \
>> +  tst-mutex6 \
>> +  tst-mutex7 \
>> +  tst-mutex9 \
>> +  tst-mutex10 \
>> +  tst-mutex11 \
>> +  tst-once1 \
>> +  tst-once2 \
>> +  tst-once3 \
>> +  tst-once4 \
>> +  tst-pt-align \
>> +  tst-pt-align3 \
>> +  tst-pt-popen1 \
>> +  tst-pt-sysconf \
>> +  tst-pt-tls1 \
>> +  tst-pt-tls2 \
>> +  tst-pt-vfork1 \
>> +  tst-pt-vfork2 \
>> +  tst-pthread-exit-signal \
>> +  tst-pthread-mutexattr \
>> +  tst-pthread-raise-blocked-self \
>> +  tst-pthread-setuid-loop \
>> +  tst-pthread_cancel-exited \
>> +  tst-pthread_cancel-select-loop \
>> +  tst-pthread_kill-exited \
>> +  tst-pthread_kill-exiting \
>> +  tst-raise1 \
>> +  tst-robust1 \
>> +  tst-robust2 \
>> +  tst-robust3 \
>> +  tst-robust4 \
>> +  tst-robust5 \
>> +  tst-robust6 \
>> +  tst-robust7 \
>> +  tst-robust9 \
>> +  tst-robust10 \
>> +  tst-robust11 \
>> +  tst-rwlock-tryrdlock-stall \
>> +  tst-rwlock-trywrlock-stall \
>> +  tst-rwlock1 \
>> +  tst-rwlock4 \
>> +  tst-rwlock5 \
>> +  tst-rwlock12 \
>> +  tst-rwlock13 \
>> +  tst-rwlock14 \
>> +  tst-rwlock16 \
>> +  tst-sem1 \
>> +  tst-sem2 \
>> +  tst-sem3 \
>> +  tst-sem4 \
>> +  tst-sem5 \
>> +  tst-sem6 \
>> +  tst-sem7 \
>> +  tst-sem8 \
>> +  tst-sem9 \
>> +  tst-sem10 \
>> +  tst-sem14 \
>> +  tst-sem15 \
>> +  tst-sem16 \
>> +  tst-setuid3 \
>> +  tst-signal1 \
>> +  tst-signal2 \
>> +  tst-signal4 \
>> +  tst-signal5 \
>> +  tst-signal6 \
>> +  tst-signal8 \
>> +  tst-spin1 \
>> +  tst-spin2 \
>> +  tst-spin3 \
>> +  tst-spin4 \
>> +  tst-stack1 \
>> +  tst-stdio1 \
>> +  tst-stdio2 \
>> +  tst-thrd-detach \
>> +  tst-thrd-sleep \
>> +  tst-tsd1 \
>> +  tst-tsd2 \
>> +  tst-tsd5 \
>> +  tst-tsd6 \
>> +  tst-tss-basic \
>> +  tst-umask1 \
>> +  tst-unload \
>> +  tst-unwind-thread \
>> +  tst-vfork1x \
>> +  tst-vfork2x \
>> +  # tests
>>     tests-time64 += \
>>     tst-abstime-time64 \
>> @@ -138,47 +291,70 @@ tests-time64 += \
>>     tst-rwlock14-time64 \
>>     tst-sem5-time64 \
>>     tst-thrd-sleep-time64 \
>> +  # tests-time64
>>     static-only-routines = pthread_atfork
>>     # Files which must not be linked with libpthread.
>> -tests-nolibpthread += tst-unload
>> +tests-nolibpthread += \
>> +  tst-unload \
>> +  # tests-nolibpthread
>>     # GCC-4.9 compiles 'sprintf(NULL, ...)' into UD2 on x86_64 without -fno-builtin
>>   CFLAGS-tst-cleanup2.c += -fno-builtin
>>   CFLAGS-tst-cleanupx2.c += -fno-builtin
>>   -tests += tst-cancelx2 tst-cancelx3 tst-cancelx6 tst-cancelx8 tst-cancelx9 \
>> -     tst-cancelx4 tst-cancelx5 \
>> -     tst-cancelx10 tst-cancelx11 tst-cancelx12 tst-cancelx13 tst-cancelx14 \
>> -     tst-cancelx15 tst-cancelx16 tst-cancelx18 tst-cancelx20 tst-cancelx21 \
>> -     tst-cleanupx0 tst-cleanupx1 tst-cleanupx2 tst-cleanupx3
>> +tests += \
>> +  tst-cancelx2 \
>> +  tst-cancelx3 \
>> +  tst-cancelx4 \
>> +  tst-cancelx5 \
>> +  tst-cancelx6 \
>> +  tst-cancelx8 \
>> +  tst-cancelx9 \
>> +  tst-cancelx10 \
>> +  tst-cancelx11 \
>> +  tst-cancelx12 \
>> +  tst-cancelx13 \
>> +  tst-cancelx14 \
>> +  tst-cancelx15 \
>> +  tst-cancelx16 \
>> +  tst-cancelx18 \
>> +  tst-cancelx20 \
>> +  tst-cancelx21 \
>> +  tst-cleanupx0 \
>> +  tst-cleanupx1 \
>> +  tst-cleanupx2 \
>> +  tst-cleanupx3 \
>> +  # tests
>>     ifeq ($(build-shared),yes)
>>   tests += \
>> -  tst-atfork2 \
>> -  tst-pt-tls4 \
>>     tst-_res1 \
>> -  tst-fini1 \
>> -  tst-create1 \
>> +  tst-atfork2 \
>>     tst-atfork3 \
>>     tst-atfork4 \
>> -# tests
>> +  tst-create1 \
>> +  tst-fini1 \
>> +  tst-pt-tls4 \
>> +  # tests
>>   -tests-nolibpthread += tst-fini1
>> +tests-nolibpthread += \
>> +  tst-fini1 \
>> +  # tests-nolibpthread
>>   endif
>>     modules-names += \
>> -  tst-atfork2mod \
>> -  tst-tls4moda \
>> -  tst-tls4modb \
>>     tst-_res1mod1 \
>>     tst-_res1mod2 \
>> -  tst-fini1mod \
>> -  tst-create1mod \
>> +  tst-atfork2mod \
>>     tst-atfork3mod \
>>     tst-atfork4mod \
>> -# module-names
>> +  tst-create1mod \
>> +  tst-fini1mod \
>> +  tst-tls4moda \
>> +  tst-tls4modb \
>> +  # modules-names
>>     test-modules = $(addprefix $(objpfx),$(addsuffix .so,$(modules-names)))
>>   @@ -192,17 +368,30 @@ ifeq ($(build-shared),yes)
>>   tests: $(test-modules)
>>   endif
>>   +tests-static += \
>> +  tst-cancel21-static \
>> +  tst-locale1 \
>> +  tst-locale2 \
>> +  # tests-static
>>   -tests-static += tst-locale1 tst-locale2 tst-cancel21-static
>> -
>> -tests += tst-cancel21-static tst-cond11-static
>> +tests += \
>> +  tst-cancel21-static \
>> +  tst-cond11-static \
>> +  # tests
>>     # These tests are linked with libc before libpthread
>> -tests-reverse += tst-cancel5 tst-cancel23 tst-vfork1x tst-vfork2x
>> +tests-reverse += \
>> +  tst-cancel5 \
>> +  tst-cancel23 \
>> +  tst-vfork1x \
>> +  tst-vfork2x \
>> +  # tests-reverse
>>     ifeq ($(run-built-tests),yes)
>>   ifeq ($(build-shared),yes)
>> -tests-special += $(objpfx)tst-cleanup0-cmp.out
>> +tests-special += \
>> +  $(objpfx)tst-cleanup0-cmp.out \
>> +  # tests-special
>>   endif
>>   endif
>>   @@ -286,20 +475,38 @@ $(objpfx)tst-_res1: $(objpfx)tst-_res1mod1.so $(objpfx)tst-_res1mod2.so \
>>   $(objpfx)tst-pt-tls4: $(shared-thread-library)
>>   $(objpfx)tst-pt-tls4.out: $(objpfx)tst-tls4moda.so $(objpfx)tst-tls4modb.so
>>   -generated += tst-atfork2.mtrace
>> +generated += \
>> +  tst-atfork2.mtrace \
>> +  # generated
>>   -generated += $(objpfx)tst-atfork2.mtrace \
>> -         $(addsuffix .so,$(strip $(modules-names)))
>> +generated += \
>> +  $(addsuffix .so,$(strip $(modules-names))) \
>> +  $(objpfx)tst-atfork2.mtrace \
>> +  # generated
>>   -tests-internal += tst-cancel25 tst-robust8
>> +tests-internal += \
>> +  tst-cancel25 \
>> +  tst-robust8 \
>> +  # tests-internal
>>   -tests += tst-oncex3 tst-oncex4 tst-oncey3 tst-oncey4
>> +tests += \
>> +  tst-oncex3 \
>> +  tst-oncex4 \
>> +  tst-oncey3 \
>> +  tst-oncey4 \
>> +  # tests
>>   -modules-names += tst-join7mod
>> +modules-names += \
>> +  tst-join7mod \
>> +  # modules-names
>>     ifeq ($(build-shared),yes)
>> -tests-static += tst-cond8-static
>> -tests += tst-cond8-static
>> +tests-static += \
>> +  tst-cond8-static \
>> +  # tests-static
>> +tests += \
>> +  tst-cond8-static \
>> +  # tests
>>   endif
>>     CFLAGS-tst-oncex3.c += -fexceptions
>> diff --git a/sysdeps/pthread/tst-mutex7robust.c b/sysdeps/pthread/tst-robust11.c
>> similarity index 100%
>> rename from sysdeps/pthread/tst-mutex7robust.c
>> rename to sysdeps/pthread/tst-robust11.c
> 

-- 
Cheers,
Carlos.


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

* Re: [PATCH v3 1/2] scripts: Add sort-makefile-lines.py to sort Makefile variables.
  2023-05-10 15:49   ` Siddhesh Poyarekar
@ 2023-05-10 17:17     ` Carlos O'Donell
  0 siblings, 0 replies; 12+ messages in thread
From: Carlos O'Donell @ 2023-05-10 17:17 UTC (permalink / raw)
  To: Siddhesh Poyarekar, libc-alpha, alx.manpages

On 5/10/23 11:49, Siddhesh Poyarekar wrote:
> On 2023-05-10 07:58, Carlos O'Donell wrote:
>> The scripts/sort-makefile-lines.py script sorts Makefile variables
>> according to project expected order.
>>
>> The script can be used like this:
>>
>> $ scripts/sort-makefile-lines.py < elf/Makefile > elf/Makefile.tmp
>> $ mv elf/Makefile.tmp elf/Makefile
>> ---
>> v2->v3: Use stdin/stdout.
>>
>>   scripts/sort-makefile-lines.py | 160 +++++++++++++++++++++++++++++++++
>>   1 file changed, 160 insertions(+)
>>   create mode 100755 scripts/sort-makefile-lines.py
> 
> LGTM.
> 
> Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>

Thanks! Pushed.
 
>>
>> diff --git a/scripts/sort-makefile-lines.py b/scripts/sort-makefile-lines.py
>> new file mode 100755
>> index 0000000000..fd657df970
>> --- /dev/null
>> +++ b/scripts/sort-makefile-lines.py
>> @@ -0,0 +1,160 @@
>> +#!/usr/bin/python3
>> +# Sort Makefile lines as expected by project policy.
>> +# Copyright (C) 2023 Free Software Foundation, Inc.
>> +# This file is part of the GNU C Library.
>> +#
>> +# The GNU C Library is free software; you can redistribute it and/or
>> +# modify it under the terms of the GNU Lesser General Public
>> +# License as published by the Free Software Foundation; either
>> +# version 2.1 of the License, or (at your option) any later version.
>> +#
>> +# The GNU C Library is distributed in the hope that it will be useful,
>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> +# Lesser General Public License for more details.
>> +#
>> +# You should have received a copy of the GNU Lesser General Public
>> +# License along with the GNU C Library; if not, see
>> +# <https://www.gnu.org/licenses/>.
>> +
>> +# The project consensus is to split Makefile variable assignment
>> +# across multiple lines with one value per line.  The values are
>> +# then sorted as described below, and terminated with a special
>> +# list termination marker.  This splitting makes it much easier
>> +# to add new tests to the list since they become just a single
>> +# line insertion.  It also makes backports and merges easier
>> +# since the new test may not conflict due to the ordering.
>> +#
>> +# Consensus discussion:
>> +# https://inbox.sourceware.org/libc-alpha/f6406204-84f5-adb1-d00e-979ebeebbbde@redhat.com/
>> +#
>> +# To support cleaning up Makefiles we created this program to
>> +# help sort existing lists converted to the new format.
>> +#
>> +# The program takes as input the Makefile to sort correctly,
>> +# and the output file to write the correctly sorted output
>> +# (it can be the same file).
>> +#
>> +# Sorting is only carried out between two special markers:
>> +# (a) Marker start is '<variable> += \' (or '= \', or ':= \')
>> +# (b) Marker end is '  # <variable>' (whitespace matters)
>> +# With everthing between (a) and (b) being sorted accordingly.
>> +#
>> +# You can use it like this:
>> +# $ scripts/sort-makefile-lines.py < elf/Makefile > elf/Makefile.tmp
>> +# $ mv elf/Makefile.tmp elf/Makefile
>> +#
>> +# The Makefile lines in the project are sorted using the
>> +# following rules:
>> +# - All lines are sorted as-if `LC_COLLATE=C sort`
>> +# - Lines that have a numeric suffix and whose leading prefix
>> +#   matches exactly are sorted according the numeric suffix
>> +#   in increasing numerical order.
>> +#
>> +# For example:
>> +# ~~~
>> +# tests += \
>> +#   test-a \
>> +#   test-b \
>> +#   test-b1 \
>> +#   test-b2 \
>> +#   test-b10 \
>> +#   test-b20 \
>> +#   test-b100 \
>> +#   # tests
>> +# ~~~
>> +# This example shows tests sorted alphabetically, followed
>> +# by a numeric suffix sort in increasing numeric order.
>> +#
>> +# Cleanups:
>> +# - Tests that end in "a" or "b" variants should be renamed to
>> +#   end in just the numerical value. For example 'tst-mutex7robust'
>> +#   should be renamed to 'tst-mutex12' (the highest numbered test)
>> +#   or 'tst-robust11' (the highest numbered test) in order to get
>> +#   reasonable ordering.
>> +# - Modules that end in "mod" or "mod1" should be renamed. For
>> +#   example 'tst-atfork2mod' should be renamed to 'tst-mod-atfork2'
>> +#   (test module for atfork2). If there are more than one module
>> +#   then they should be named with a suffix that uses [0-9] first
>> +#   then [A-Z] next for a total of 36 possible modules per test.
>> +#   No manually listed test currently uses more than that (though
>> +#   automatically generated tests may; they don't need sorting).
>> +# - Avoid including another test and instead refactor into common
>> +#   code with all tests including hte common code, then give the
>> +#   tests unique names.
>> +#
>> +# If you have a Makefile that needs converting, then you can
>> +# quickly split the values into one-per-line, ensure the start
>> +# and end markers are in place, and then run the script to
>> +# sort the values.
>> +
>> +import sys
>> +import locale
>> +import re
>> +import functools
>> +
>> +def glibc_makefile_numeric(string1, string2):
>> +    # Check if string1 has a numeric suffix.
>> +    var1 = re.search(r'([0-9]+) \\$', string1)
>> +    var2 = re.search(r'([0-9]+) \\$', string2)
>> +    if var1 and var2:
>> +        if string1[0:var1.span()[0]] == string2[0:var2.span()[0]]:
>> +            # string1 and string2 both share a prefix and
>> +            # have a numeric suffix that can be compared.
>> +            # Sort order is based on the numeric suffix.
>> +            return int(var1.group(1)) > int(var2.group(1))
>> +    # Default to strcoll.
>> +    return locale.strcoll(string1, string2)
>> +
>> +def sort_lines(lines):
>> +
>> +    # Use the C locale for language independent collation.
>> +    locale.setlocale (locale.LC_ALL, "C")
>> +
>> +    # Sort using a glibc-specific sorting function.
>> +    lines = sorted(lines, key=functools.cmp_to_key(glibc_makefile_numeric))
>> +
>> +    return lines
>> +
>> +def sort_makefile_lines():
>> +
>> +    # Read the whole Makefile.
>> +    lines = sys.stdin.readlines()
>> +
>> +    # Build a list of all start markers (tuple includes name).
>> +    startmarks = []
>> +    for i in range(len(lines)):
>> +        # Look for things like "var = \", "var := \" or "var += \"
>> +        # to start the sorted list.
>> +        var = re.search(r'^([a-zA-Z0-9-]*) [\+:]?\= \\$', lines[i])
>> +        if var:
>> +            # Remember the index and the name.
>> +            startmarks.append((i, var.group(1)))
>> +
>> +    # For each start marker try to find a matching end mark
>> +    # and build a block that needs sorting.  The end marker
>> +    # must have the matching comment name for it to be valid.
>> +    rangemarks = []
>> +    for sm in startmarks:
>> +        # Look for things like "  # var" to end the sorted list.
>> +        reg = r'^  # ' + sm[1] + r'$'
>> +        for j in range(sm[0] + 1, len(lines)):
>> +            if re.search(reg, lines[j]):
>> +                # Rembember the block to sort (inclusive).
>> +                rangemarks.append((sm[0] + 1, j))
>> +                break
>> +
>> +    # We now have a list of all ranges that need sorting.
>> +    # Sort those ranges (inclusive).
>> +    for r in rangemarks:
>> +        lines[r[0]:r[1]] = sort_lines(lines[r[0]:r[1]])
>> +
>> +    # Output the whole list with sorted lines to stdout.
>> +    [sys.stdout.write(line) for line in lines]
>> +
>> +
>> +def main(argv):
>> +    sort_makefile_lines ()
>> +
>> +if __name__ == '__main__':
>> +    main(sys.argv[1:])
> 

-- 
Cheers,
Carlos.


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

* Re: [PATCH v3 1/2] scripts: Add sort-makefile-lines.py to sort Makefile variables.
  2023-05-10 15:12       ` Alejandro Colomar
@ 2023-05-10 17:30         ` Carlos O'Donell
  2023-05-10 18:33           ` Alejandro Colomar
  0 siblings, 1 reply; 12+ messages in thread
From: Carlos O'Donell @ 2023-05-10 17:30 UTC (permalink / raw)
  To: Alejandro Colomar, libc-alpha, siddhesh

On 5/10/23 11:12, Alejandro Colomar wrote:
> Hi Carlos,
> 
> On 5/10/23 17:07, Carlos O'Donell wrote:
>> On 5/10/23 08:42, Alejandro Colomar wrote:
>>> I believe you're looking for a version sort(1).
>>
>> Yes, sort -V does work, the downside is the integration to handle the blocks
>> of text in the Makefile.
> 
> Yup, I've been thinking about it, and a pipeline seems non-obvious.
> It would need some bash magic, or maybe some non-trivial awk(1) or perl(1).

Exactly, I'd use bash arrays.

>>
>>> I don't know how easy it is to call sort(1) in this script, but it
>>> would probably simplify a big part of it.  And it would also make
>>> unnecessary the renaming of things like 'tst-mutex7robust'.
>>
>> You don't *need* to rename any tests in the new version of the code, I took
>> out the error case and just sort as the new function sorts (like sort -V does).
>>
>>> If using `sort -V` here is complex, maybe it's easier to write a
>>> shell script.
>>
>> I would want to use bash with arrays to handle lines and processing the start
>> and end blocks. It could be possible to do it in an equivalent number of lines,
>> but I think the standalone python is easier to read and maintain (less shell
>> quoting issues with lines of text).
>>
>> Any objection to the proposed python?
> 
> Not really.  I was just suggesting it case it didn't occur to you.
> Maybe a small objection would be in the naming of the functions.
> If it sorts as if `sort -V`, how about calling it something like
> version_sort?  And maybe a comment stating it behaves as if `sort -V`?

It doesn't sort exactly like `sort -V` because of the dashes.

$ cat sort-dash.txt 
0
-
1
2
$ LC_COLLATE=C sort sort-dash.txt 
-
0
1
2
$ sort -V sort-dash.txt 
0
1
2
-

Consensus was to use something *like* `LC_COLLATE=C sort` (code point sort order) but
with trailing numeric sort.

The above shows that `sort -V` treats U+002D ('-') differently from the code-point sort
order and sorts it after U+0030 ('0')

Coreutils itself identifies there is no standard version sort:
https://www.gnu.org/software/coreutils/manual/html_node/Variations-in-version-sort-order.html

With the python implementation we get a stable sort that the project agrees is the way to
sort variables.

Lastly I want to avoid dependencies on things like Python's natsort, which are outside the
minimum python we use in glibc to facilitate a bootstrap.

> Other than that, I'm fine with it :)

Thank you again for looking it over. Your suggestions helped me delete code and make it
simpler overall!

-- 
Cheers,
Carlos.


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

* Re: [PATCH v3 1/2] scripts: Add sort-makefile-lines.py to sort Makefile variables.
  2023-05-10 17:30         ` Carlos O'Donell
@ 2023-05-10 18:33           ` Alejandro Colomar
  0 siblings, 0 replies; 12+ messages in thread
From: Alejandro Colomar @ 2023-05-10 18:33 UTC (permalink / raw)
  To: Carlos O'Donell, libc-alpha, siddhesh


[-- Attachment #1.1: Type: text/plain, Size: 1886 bytes --]

Hi Carlos,

On 5/10/23 19:30, Carlos O'Donell wrote:
[...]

>> Yup, I've been thinking about it, and a pipeline seems non-obvious.
>> It would need some bash magic, or maybe some non-trivial awk(1) or perl(1).
> 
> Exactly, I'd use bash arrays.
> 

[...]

>>> Any objection to the proposed python?
>>
>> Not really.  I was just suggesting it case it didn't occur to you.
>> Maybe a small objection would be in the naming of the functions.
>> If it sorts as if `sort -V`, how about calling it something like
>> version_sort?  And maybe a comment stating it behaves as if `sort -V`?
> 
> It doesn't sort exactly like `sort -V` because of the dashes.
> 
> $ cat sort-dash.txt 
> 0
> -
> 1
> 2
> $ LC_COLLATE=C sort sort-dash.txt 
> -
> 0
> 1
> 2
> $ sort -V sort-dash.txt 
> 0
> 1
> 2
> -
> 
> Consensus was to use something *like* `LC_COLLATE=C sort` (code point sort order) but
> with trailing numeric sort.
> 
> The above shows that `sort -V` treats U+002D ('-') differently from the code-point sort
> order and sorts it after U+0030 ('0')
> 
> Coreutils itself identifies there is no standard version sort:
> https://www.gnu.org/software/coreutils/manual/html_node/Variations-in-version-sort-order.html
> 
> With the python implementation we get a stable sort that the project agrees is the way to
> sort variables.
> 
> Lastly I want to avoid dependencies on things like Python's natsort, which are outside the
> minimum python we use in glibc to facilitate a bootstrap.

Sounds reasonable.

> 
>> Other than that, I'm fine with it :)
> 
> Thank you again for looking it over. Your suggestions helped me delete code and make it
> simpler overall!

I'm very glad to read that.  Thank you!  :-)

Cheers,
Alex

> 

-- 
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

end of thread, other threads:[~2023-05-10 18:33 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-10 11:58 [PATCH v3 0/2] Standardize the sorting of Makefile variables Carlos O'Donell
2023-05-10 11:58 ` [PATCH v3 1/2] scripts: Add sort-makefile-lines.py to sort " Carlos O'Donell
2023-05-10 12:42   ` Alejandro Colomar
2023-05-10 15:07     ` Carlos O'Donell
2023-05-10 15:12       ` Alejandro Colomar
2023-05-10 17:30         ` Carlos O'Donell
2023-05-10 18:33           ` Alejandro Colomar
2023-05-10 15:49   ` Siddhesh Poyarekar
2023-05-10 17:17     ` Carlos O'Donell
2023-05-10 11:58 ` [PATCH v3 2/2] nptl: Reformat Makefile Carlos O'Donell
2023-05-10 13:01   ` Siddhesh Poyarekar
2023-05-10 17:16     ` Carlos O'Donell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).