Message ID | 20240522005913.3540131-1-edliaw@google.com (mailing list archive) |
---|---|
Headers | show |
Series | Define _GNU_SOURCE for sources using | expand |
On Wed, 2024-05-22 at 00:56 +0000, Edward Liaw wrote: > Centralizes the definition of _GNU_SOURCE into KHDR_INCLUDES and removes > redefinitions of _GNU_SOURCE from source code. > > 809216233555 ("selftests/harness: remove use of LINE_MAX") introduced > asprintf into kselftest_harness.h, which is a GNU extension and needs > _GNU_SOURCE to either be defined prior to including headers or with the > -D_GNU_SOURCE flag passed to the compiler. I'm sorry for the late question, but what is the merge plan here? Thanks! Paolo
On 5/21/24 18:56, Edward Liaw wrote: > Centralizes the definition of _GNU_SOURCE into KHDR_INCLUDES and removes > redefinitions of _GNU_SOURCE from source code. > > 809216233555 ("selftests/harness: remove use of LINE_MAX") introduced > asprintf into kselftest_harness.h, which is a GNU extension and needs Easier solution to define LINE_MAX locally. In gerenal it is advisable to not add local defines, but it is desirable in some cases to avoid churn like this one. > _GNU_SOURCE to either be defined prior to including headers or with the > -D_GNU_SOURCE flag passed to the compiler. > This is huge churn to all the tests and some maintainers aren't onboard to take this change. Is there an wasier way to fix this instead? Please explore localized options before asking me to take this series. thanks, -- Shuah
On 5/22/24 01:42, Paolo Abeni wrote: > On Wed, 2024-05-22 at 00:56 +0000, Edward Liaw wrote: >> Centralizes the definition of _GNU_SOURCE into KHDR_INCLUDES and removes >> redefinitions of _GNU_SOURCE from source code. >> >> 809216233555 ("selftests/harness: remove use of LINE_MAX") introduced >> asprintf into kselftest_harness.h, which is a GNU extension and needs >> _GNU_SOURCE to either be defined prior to including headers or with the >> -D_GNU_SOURCE flag passed to the compiler. > > I'm sorry for the late question, but what is the merge plan here? > I have asked Edward Liaw explore options to localize the fix to the problem introduced by the following commit 809216233555 ("selftests/harness: remove use of LINE_MAX") I am not happy with the churn. I don't plan to merge this series as it for sure. If and when this problem gets fixed, I plan to merge the change and take it through kselftest. thanks, -- Shuah
On Wed, 22 May 2024 10:19:33 -0600 Shuah Khan wrote: > On 5/21/24 18:56, Edward Liaw wrote: > > Centralizes the definition of _GNU_SOURCE into KHDR_INCLUDES and removes > > redefinitions of _GNU_SOURCE from source code. > > > > 809216233555 ("selftests/harness: remove use of LINE_MAX") introduced > > asprintf into kselftest_harness.h, which is a GNU extension and needs > > Easier solution to define LINE_MAX locally. In gerenal it is advisable > to not add local defines, but it is desirable in some cases to avoid > churn like this one. Will the patch that Andrew applied: https://lore.kernel.org/all/20240519213733.2AE81C32781@smtp.kernel.org/ make its way to Linus? As you say that's a much simpler fix.
On Wed, May 22, 2024 at 10:13 AM Jakub Kicinski <kuba@kernel.org> wrote: > > On Wed, 22 May 2024 10:19:33 -0600 Shuah Khan wrote: > > On 5/21/24 18:56, Edward Liaw wrote: > > > Centralizes the definition of _GNU_SOURCE into KHDR_INCLUDES and removes > > > redefinitions of _GNU_SOURCE from source code. > > > > > > 809216233555 ("selftests/harness: remove use of LINE_MAX") introduced > > > asprintf into kselftest_harness.h, which is a GNU extension and needs > > > > Easier solution to define LINE_MAX locally. In gerenal it is advisable > > to not add local defines, but it is desirable in some cases to avoid > > churn like this one. > > Will the patch that Andrew applied: > https://lore.kernel.org/all/20240519213733.2AE81C32781@smtp.kernel.org/ > make its way to Linus? As you say that's a much simpler fix. Right, this patch series may be unnecessary after all, since the problem is fixed by that patch. It might be better to drop the series unless it is desirable to centralize the declaration of _GNU_SOURCE to the root Makefile / lib.mk. If that is still wanted, maybe a more palatable approach would be to surround every instance of #define _GNU_SOURCE with #ifndef _GNU_SOURCE first, then induce the change to CFLAGS in lib.mk. That would prevent a partial merge from triggering build warnings.
On 5/22/24 11:44, Edward Liaw wrote: > On Wed, May 22, 2024 at 10:13 AM Jakub Kicinski <kuba@kernel.org> wrote: >> >> On Wed, 22 May 2024 10:19:33 -0600 Shuah Khan wrote: >>> On 5/21/24 18:56, Edward Liaw wrote: >>>> Centralizes the definition of _GNU_SOURCE into KHDR_INCLUDES and removes >>>> redefinitions of _GNU_SOURCE from source code. >>>> >>>> 809216233555 ("selftests/harness: remove use of LINE_MAX") introduced >>>> asprintf into kselftest_harness.h, which is a GNU extension and needs >>> >>> Easier solution to define LINE_MAX locally. In gerenal it is advisable >>> to not add local defines, but it is desirable in some cases to avoid >>> churn like this one. >> >> Will the patch that Andrew applied: >> https://lore.kernel.org/all/20240519213733.2AE81C32781@smtp.kernel.org/ >> make its way to Linus? As you say that's a much simpler fix. > Thank you Jakub. Yes. This is a simpler fix. > Right, this patch series may be unnecessary after all, since the > problem is fixed by that patch. > > It might be better to drop the series unless it is desirable to > centralize the declaration of _GNU_SOURCE to the root Makefile / > lib.mk. If that is still wanted, maybe a more palatable approach > would be to surround every instance of #define _GNU_SOURCE with > #ifndef _GNU_SOURCE first, then induce the change to CFLAGS in lib.mk. > That would prevent a partial merge from triggering build warnings. Please drop this series. thanks, -- Shuah
Hello: This series was applied to riscv/linux.git (fixes) by Tejun Heo <tj@kernel.org>: On Wed, 22 May 2024 00:56:46 +0000 you wrote: > Centralizes the definition of _GNU_SOURCE into KHDR_INCLUDES and removes > redefinitions of _GNU_SOURCE from source code. > > 809216233555 ("selftests/harness: remove use of LINE_MAX") introduced > asprintf into kselftest_harness.h, which is a GNU extension and needs > _GNU_SOURCE to either be defined prior to including headers or with the > -D_GNU_SOURCE flag passed to the compiler. > > [...] Here is the summary with links: - [v5,01/68] selftests: Compile with -D_GNU_SOURCE when including lib.mk (no matching commit) - [v5,02/68] kselftest: Desecalate reporting of missing _GNU_SOURCE (no matching commit) - [v5,03/68] selftests/arm64: Drop define _GNU_SOURCE (no matching commit) - [v5,04/68] selftests/arm64: Drop duplicate -D_GNU_SOURCE (no matching commit) - [v5,05/68] selftests/bpf: Drop define _GNU_SOURCE (no matching commit) - [v5,06/68] selftests/breakpoints: Drop define _GNU_SOURCE (no matching commit) - [v5,07/68] selftests/cachestat: Drop define _GNU_SOURCE (no matching commit) - [v5,08/68] selftests/capabilities: Drop define _GNU_SOURCE (no matching commit) - [v5,09/68] selftests/cgroup: Drop define _GNU_SOURCE https://git.kernel.org/riscv/c/c1457d9aad5e - [v5,10/68] selftests/clone3: Drop define _GNU_SOURCE (no matching commit) - [v5,11/68] selftests/core: Drop define _GNU_SOURCE (no matching commit) - [v5,12/68] selftests/damon: Drop define _GNU_SOURCE (no matching commit) - [v5,13/68] selftests/drivers: Drop define _GNU_SOURCE (no matching commit) - [v5,14/68] selftests/exec: Drop duplicate -D_GNU_SOURCE (no matching commit) - [v5,15/68] selftests/fchmodat2: Drop define _GNU_SOURCE (no matching commit) - [v5,16/68] selftests/filelock: Drop define _GNU_SOURCE (no matching commit) - [v5,17/68] selftests/filesystems: Drop define _GNU_SOURCE (no matching commit) - [v5,18/68] selftests/firmware: Drop define _GNU_SOURCE (no matching commit) - [v5,19/68] selftests/fpu: Drop define _GNU_SOURCE (no matching commit) - [v5,20/68] selftests/futex: Drop define _GNU_SOURCE (no matching commit) - [v5,21/68] selftests/futex: Drop duplicate -D_GNU_SOURCE (no matching commit) - [v5,22/68] selftests/intel_pstate: Drop duplicate -D_GNU_SOURCE (no matching commit) - [v5,23/68] selftests/iommu: Drop duplicate -D_GNU_SOURCE (no matching commit) - [v5,24/68] selftests/ipc: Drop define _GNU_SOURCE (no matching commit) - [v5,25/68] selftests/kcmp: Drop define _GNU_SOURCE (no matching commit) - [v5,26/68] selftests/landlock: Drop define _GNU_SOURCE (no matching commit) - [v5,27/68] selftests/lsm: Drop define _GNU_SOURCE (no matching commit) - [v5,28/68] selftests/membarrier: Drop define _GNU_SOURCE (no matching commit) - [v5,29/68] selftests/memfd: Drop define _GNU_SOURCE (no matching commit) - [v5,30/68] selftests/mincore: Drop define _GNU_SOURCE (no matching commit) - [v5,31/68] selftests/mm: Drop define _GNU_SOURCE (no matching commit) - [v5,32/68] selftests/mount: Drop define _GNU_SOURCE (no matching commit) - [v5,33/68] selftests/mount_setattr: Drop define _GNU_SOURCE (no matching commit) - [v5,34/68] selftests/move_mount_set_group: Drop define _GNU_SOURCE (no matching commit) - [v5,35/68] selftests/mqueue: Drop define _GNU_SOURCE (no matching commit) - [v5,36/68] selftests/net: Drop define _GNU_SOURCE (no matching commit) - [v5,37/68] selftests/net: Drop duplicate -D_GNU_SOURCE (no matching commit) - [v5,38/68] selftests/nsfs: Drop define _GNU_SOURCE (no matching commit) - [v5,39/68] selftests/openat2: Drop define _GNU_SOURCE (no matching commit) - [v5,40/68] selftests/perf_events: Drop define _GNU_SOURCE (no matching commit) - [v5,41/68] selftests/pid_namespace: Drop define _GNU_SOURCE (no matching commit) - [v5,42/68] selftests/pidfd: Drop define _GNU_SOURCE (no matching commit) - [v5,43/68] selftests/ptrace: Drop define _GNU_SOURCE (no matching commit) - [v5,44/68] selftests/powerpc: Drop define _GNU_SOURCE (no matching commit) - [v5,45/68] selftests/proc: Drop define _GNU_SOURCE (no matching commit) - [v5,46/68] selftests/proc: Drop duplicate -D_GNU_SOURCE (no matching commit) - [v5,47/68] selftests/ptp: Drop define _GNU_SOURCE (no matching commit) - [v5,48/68] selftests/resctrl: Drop duplicate -D_GNU_SOURCE (no matching commit) - [v5,49/68] selftests/riscv: Drop define _GNU_SOURCE (no matching commit) - [v5,50/68] selftests/riscv: Drop duplicate -D_GNU_SOURCE (no matching commit) - [v5,51/68] selftests/rlimits: Drop define _GNU_SOURCE (no matching commit) - [v5,52/68] selftests/rseq: Drop define _GNU_SOURCE (no matching commit) - [v5,53/68] selftests/safesetid: Drop define _GNU_SOURCE (no matching commit) - [v5,54/68] selftests/sched: Drop define _GNU_SOURCE (no matching commit) - [v5,55/68] selftests/seccomp: Drop define _GNU_SOURCE (no matching commit) - [v5,56/68] selftests/sigaltstack: Drop define _GNU_SOURCE (no matching commit) - [v5,57/68] selftests/sgx: Compile with -D_GNU_SOURCE (no matching commit) - [v5,58/68] selftests/splice: Drop define _GNU_SOURCE (no matching commit) - [v5,59/68] selftests/syscall_user_dispatch: Drop define _GNU_SOURCE (no matching commit) - [v5,60/68] selftests/thermal: Drop define _GNU_SOURCE (no matching commit) - [v5,61/68] selftests/timens: Drop define _GNU_SOURCE (no matching commit) - [v5,62/68] selftests/tmpfs: Drop duplicate -D_GNU_SOURCE (no matching commit) - [v5,63/68] selftests/uevent: Drop define _GNU_SOURCE (no matching commit) - [v5,64/68] selftests/user_events: Drop define _GNU_SOURCE (no matching commit) - [v5,65/68] selftests/vDSO: Append to CFLAGS in Makefile (no matching commit) - [v5,66/68] selftests/vDSO: Drop define _GNU_SOURCE (no matching commit) - [v5,67/68] selftests/x86: Append to CFLAGS in Makefile (no matching commit) - [v5,68/68] selftests/x86: Drop define _GNU_SOURCE (no matching commit) You are awesome, thank you!
On 5/22/24 17:32, patchwork-bot+linux-riscv@kernel.org wrote: > Hello: > > This series was applied to riscv/linux.git (fixes) > by Tejun Heo <tj@kernel.org>: > > On Wed, 22 May 2024 00:56:46 +0000 you wrote: >> Centralizes the definition of _GNU_SOURCE into KHDR_INCLUDES and removes >> redefinitions of _GNU_SOURCE from source code. >> >> 809216233555 ("selftests/harness: remove use of LINE_MAX") introduced >> asprintf into kselftest_harness.h, which is a GNU extension and needs >> _GNU_SOURCE to either be defined prior to including headers or with the >> -D_GNU_SOURCE flag passed to the compiler. Hi Tejun, Please don't. We determined this series is no longer necessary. With the patch that Andrew applied: https://lore.kernel.org/all/20240519213733.2AE81C32781@smtp.kernel.org/ make its way to Linus? As you say that's a much simpler fix. thanks, -- Shuah
On 5/22/24 17:36, Shuah Khan wrote: > On 5/22/24 17:32, patchwork-bot+linux-riscv@kernel.org wrote: >> Hello: >> >> This series was applied to riscv/linux.git (fixes) >> by Tejun Heo <tj@kernel.org>: >> Hi Tejun, I noticed you weren't on the email I sent in response. Please drop this series. There is simpler fix to the problem this patch series attempts to solve with this series is already in Linus's tree: https://lore.kernel.org/all/20240519213733.2AE81C32781@smtp.kernel.org/ >> On Wed, 22 May 2024 00:56:46 +0000 you wrote: >>> Centralizes the definition of _GNU_SOURCE into KHDR_INCLUDES and removes >>> redefinitions of _GNU_SOURCE from source code. >>> >>> 809216233555 ("selftests/harness: remove use of LINE_MAX") introduced >>> asprintf into kselftest_harness.h, which is a GNU extension and needs >>> _GNU_SOURCE to either be defined prior to including headers or with the >>> -D_GNU_SOURCE flag passed to the compiler. > > Hi Tejun, > > Please don't. We determined this series is no longer necessary. > > With the patch that Andrew applied: > https://lore.kernel.org/all/20240519213733.2AE81C32781@smtp.kernel.org/ > make its way to Linus? As you say that's a much simpler fix. > This patch series isn't necessary and makes it problematic because all these patches are labeled as fixes - I don't plan upon taking this series. thanks, -- Shuah