dlang / dub

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

[v1.40.0] Remove code deprecated in v1.30.0 #2924

Open Geod24 opened 3 weeks ago

Geod24 commented 3 weeks ago

Release v1.30.0 was a massive release (337 commits).

The main changes were:

Once v1.39.0 is released, we can merge this PR and get rid of a lot of deprecated code, which will also massively simplify future work.

github-actions[bot] commented 3 weeks ago

BUILD FAILED ❌ Basic dub build failed! Please check your changes again.

Build statistics:

 statistics (-before, +after)
 executable size=5259464 bin/dub
-rough build time=64s
+rough build time=4s
Full build output ``` DUB version 1.37.0, built on May 11 2024 LDC - the LLVM D compiler (1.38.0): based on DMD v2.108.1 and LLVM 18.1.5 built with LDC - the LLVM D compiler (1.38.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 spirv - SPIR-V Logical 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.38.0/x64/ldc2-1.38.0-linux-x86_64/bin/ldc2 for x86_64. Building dub 1.38.0+commit.71.gc38b74c2: building configuration [application] source/dub/packagemanager.d(895,14): Error: no property `loadOverrides` for `repository` of type `dub.packagemanager.Location` source/dub/packagemanager.d(1160,9): struct `Location` defined here Error /opt/hostedtoolcache/dc/ldc2-1.38.0/x64/ldc2-1.38.0-linux-x86_64/bin/ldc2 failed with exit code 1. BUILD FAILED STAT:statistics (-before, +after) STAT:executable size=5259464 bin/dub STAT:rough build time=4s ```
Geod24 commented 3 weeks ago

This will definitely require a 2.0.0 version bump!

So we don't follow standard D policy of 10 releases ? It feels odd to have a stronger policy than the compiler. I get your point, and it's the correct thing to do, I don't know if it's the right one though.

In general, as a user, I absolutely hate this kind of light-hearted process for cleaning up/changing APIs that prioritizes minimal improvements on the library side over the constant need to follow these changes throughout the user base

I get that point, and we've definitely been guilty of doing this a bit too much in D. In the case of Dub, there was a lot of exploration needed to narrow down the scope of what the tool was doing. My favorite example was turning Dependency into a discriminated union: before that work, it was not possible to assess the fitness of an API. Dub.fetch(string, Dependency) was the most used API, but obviously it is too broad and that had to be reworked. Using the more user-friendly approach would probably have had an enormous upfront cost and not yielded as good a result. Note that I used what I consider a good middle ground: I did incremental changes locally, and when I was confident enough I was going in the right direction, I submitted a flurry of PR with small changes.

Note that if we want to cut a v2.0 release, I can prepare that, but I would like some more changes in then.

s-ludwig commented 3 weeks ago

So we don't follow standard D policy of 10 releases ? It feels odd to have a stronger policy than the compiler. I get your point, and it's the correct thing to do, I don't know if it's the right one though.

We can follow the same time line, just the version scheme is defined as SemVer and should be treated accordingly.

Using the more user-friendly approach would probably have had an enormous upfront cost and not yielded as good a result.

Yes, there is definitely a trade-off and many of the changes here are not problematic anyway.

Note that if we want to cut a v2.0 release, I can prepare that, but I would like some more changes in then.

Definitely makes sense to collect as many breaking changes as possible, as long as it doesn't add considerable amounts of additional development costs. I wouldn't necessarily treat this like you would treat a typical product version (i.e. the new shiny version two is out!), but simply as an incremental change that collects a bunch of breaking changes to avoid too frequent breakage.

Geod24 commented 2 weeks ago

We can follow the same time line, just the version scheme is defined as SemVer and should be treated accordingly.

To me it doesn't really make sense though, as the release would not remove all the deprecated code, just some. So I'd rather delay things if that's the case (although removing overrides will simplify things quite a lot).

I wouldn't necessarily treat this like you would treat a typical product version (i.e. the new shiny version two is out!), but simply as an incremental change that collects a bunch of breaking changes to avoid too frequent breakage.

Another possible approach would be to start a v2 branch and rebase it from time to time until it's ready to be released. But since I'm the only person that would work on it, I can just keep it locally.

atilaneves commented 2 weeks ago

Removing deprecated code means changing the API - I agree that the version should be bumped to 2.