dlang / dub

Package and build management system for D
MIT License
677 stars 227 forks source link

Let user $DFLAGS override build settings as much as possible #2796

Closed the-horo closed 10 months ago

the-horo commented 10 months ago

This change is mostly about being able to build programs without -Werror or -w.

dub setting up so that warnings are errors by default can be nice for developers making their code more correct but it is useless for consumers. Currently, there is no way to disable that without modifying the project configuration to add allowWarnings to buildRequirements. Users should, at least, be able to add -wi to DFLAGS and have the preference respected.

This isn't so much a problem with dmd or ldc in my experience as they mostly report deprecations, the problem is with gdc where every deprecation is a warning and the -Werror makes builds fail where they otherwise should have succeeded with only some warnings.

github-actions[bot] commented 10 months ago

✅ PR OK, no changes in deprecations or warnings

Total deprecations: 0

Total warnings: 0

Build statistics:

 statistics (-before, +after)
-executable size=5228376 bin/dub
-rough build time=62s
+executable size=5232472 bin/dub
+rough build time=61s
Full build output ``` DUB version 1.35.1, built on Jan 6 2024 LDC - the LLVM D compiler (1.36.0): based on DMD v2.106.1 and LLVM 17.0.6 built with LDC - the LLVM D compiler (1.36.0) Default target: x86_64-unknown-linux-gnu Host CPU: znver3 http://dlang.org - http://wiki.dlang.org/LDC Registered Targets: aarch64 - AArch64 (little endian) aarch64_32 - AArch64 (little endian ILP32) aarch64_be - AArch64 (big endian) amdgcn - AMD GCN GPUs arm - ARM arm64 - ARM64 (little endian) arm64_32 - ARM64 (little endian ILP32) armeb - ARM (big endian) avr - Atmel AVR Microcontroller bpf - BPF (host endian) bpfeb - BPF (big endian) bpfel - BPF (little endian) hexagon - Hexagon lanai - Lanai loongarch32 - 32-bit LoongArch loongarch64 - 64-bit LoongArch mips - MIPS (32-bit big endian) mips64 - MIPS (64-bit big endian) mips64el - MIPS (64-bit little endian) mipsel - MIPS (32-bit little endian) msp430 - MSP430 [experimental] nvptx - NVIDIA PTX 32-bit nvptx64 - NVIDIA PTX 64-bit ppc32 - PowerPC 32 ppc32le - PowerPC 32 LE ppc64 - PowerPC 64 ppc64le - PowerPC 64 LE r600 - AMD GPUs HD2XXX-HD6XXX riscv32 - 32-bit RISC-V riscv64 - 64-bit RISC-V sparc - Sparc sparcel - Sparc LE sparcv9 - Sparc V9 spirv32 - SPIR-V 32-bit spirv64 - SPIR-V 64-bit systemz - SystemZ thumb - Thumb thumbeb - Thumb (big endian) ve - VE wasm32 - WebAssembly 32-bit wasm64 - WebAssembly 64-bit x86 - 32-bit X86: Pentium-Pro and above x86-64 - 64-bit X86: EM64T and AMD64 xcore - XCore Upgrading project in /home/runner/work/dub/dub/ Starting Performing "release" build using /opt/hostedtoolcache/dc/ldc2-1.36.0/x64/ldc2-1.36.0-linux-x86_64/bin/ldc2 for x86_64. Building dub 1.36.0-beta.1+commit.37.gebde196d: building configuration [application] Linking dub STAT:statistics (-before, +after) STAT:executable size=5232472 bin/dub STAT:rough build time=61s ```
Geod24 commented 10 months ago

Currently you can work around this by using dflags on dependencies. It isn't pretty but it works. GDC should really treat deprecations and warnings differently though.

the-horo commented 10 months ago

Currently you can work around this by using dflags on dependencies. It isn't pretty but it works.

Can you explain me how this solution would go as I didn't know you could do that?

Geod24 commented 10 months ago
 "dependencies": {
        "vibe-d":           { "version": "~>0.9", "dflags" : [ "-preview=in", "-revert=dtorfields" ] }
    }

It will apply transitively, so eventcore, vibe-core, etc... will have those DFLAGS defined. Not ideal but until we take the time to untangle how we want the build system to look like, this is the "best" way to do it.

the-horo commented 10 months ago
 "dependencies": {
        "vibe-d":           { "version": "~>0.9", "dflags" : [ "-preview=in", "-revert=dtorfields" ] }
    }

It will apply transitively, so eventcore, vibe-core, etc... will have those DFLAGS defined. Not ideal but until we take the time to untangle how we want the build system to look like, this is the "best" way to do it.

My issue is more of the flags being put in the wrong place and overwriten rather then not applying transitively:

$ DFLAGS='-Wno-error' dub -v build --compiler=gdc
...
    Building dub 1.36.0-beta.1+commit.37.gf86ed3e9: building configuration [application]
gdc -Wno-error -o <cache>/dub -Werror -Wall -fversion=DubUseCurl -fversion=DubApplication ...

The same thing happens if I put dflags "-Wno-error" in dub.sdl.

There should be a way to have flags be passed later on the command line so that their values are respected, that's what I'm trying to fix.

Geod24 commented 10 months ago

Have you tried "buildRequirements": [ "allowWarnings" ] ?

PetarKirov commented 10 months ago

@Geod24 I think there is a common use case which Dub doesn't support very well. While developers working on a given project may find that adding "buildRequirements": [ "allowWarnings" ] to their dub.sdl/json may be the ideal approach, the same is not true for consumers of projects which ideally should not need to modify the source code of any of their dependencies. E.g. imagine a user of Linux distro that wants to rebuild their entire system for their exact CPU. Many of them would like the simplicity exporting CFLAGS=-march=native and hitting "rebuild all".

the-horo commented 10 months ago

Have you tried "buildRequirements": [ "allowWarnings" ] ?

Yes, that fixes it, but my issue is not about having a way to disable it, it's having an easy-access way to do it. Editing the dub.* file I don't consider enough. You can also consider the second issue of having user flags be respected more, which I think is always nice.

@Geod24 I think there is a common use case which Dub doesn't support very well. While developers working on a given project may find that adding "buildRequirements": [ "allowWarnings" ] to their dub.sdl/json may be the ideal approach, the same is not true for consumers of projects which ideally should not need to modify the source code of any of their dependencies. E.g. imagine a user of Linux distro that wants to rebuild their entire system for their exact CPU. Many of them would like the simplicity exporting CFLAGS=-march=native and hitting "rebuild all".

I agree with this. Gentoo, for which I maintain D packages, does require disabling -Werror: https://devmanual.gentoo.org/ebuild-writing/common-mistakes/index.html#-werror-compiler-flag-not-removed.

Geod24 commented 10 months ago

@the-horo : I can see the use case being useful, I myself had my share of similar issues. I'm starting to think the way we currently do DFLAGS does not make much sense, as it's its own build type. Anyway, looking at the code with a fresh pair of eyes, I think we should go ahead with it.