Skip to content

Respect skipRegistry value from configuration files #2903

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
dlang-bot merged 1 commit into from
Apr 28, 2024
Merged

Respect skipRegistry value from configuration files #2903

dlang-bot merged 1 commit into from
Apr 28, 2024

Conversation

@the-horo
Copy link
Contributor

@the-horo the-horo commented Apr 27, 2024

Currently, the skipRegistry value in configuration files is ignored, the final value is either the one the user passed on the command line, which is the intended behavior, or the value none if the user didn't pass a --skip-registry argument.

This commit fixes this by introducing the new value default_ to the SkippackageSuppliers enum to know when the user didn't pass the skip-registry argument and the value should be taken from the configuration files.

Be aware that the output of an invalid skipRegistry value in dub.settings.json has changed:
Before this PR:

     Warning SkipPackageSuppliers does not have a member named 'blah'

After this PR:

     Warning /tmp/test/dub.settings.json(1:17): skipRegistry: SkipPackageSuppliers does not have a member named 'blah'

@the-horo
Respect skipRegistry value from configuration files
974afbf 
Currently, the skipRegistry value in configuration files is ignored,
the final value is either the one the user passed on the command line,
which is the intended behavior, or the value `none` if the user didn't
pass a --skip-registry argument.

This commit fixes this by introducing the new value `default_` to the
SkippackageSuppliers enum to know when the user didn't pass the
skip-registry argument and the value should be taken from the
configuration files.

Signed-off-by: Andrei Horodniceanu <[email protected]>
@github-actions
Copy link

github-actions bot commented Apr 27, 2024 •
edited
Loading

✅ PR OK, no changes in deprecations or warnings

Total deprecations: 0

Total warnings: 0

Build statistics:

 statistics (-before, +after)
 executable size=5364248 bin/dub
 rough build time=64s
Full build output 
DUB version 1.36.0, built on Mar  3 2024
LDC - the LLVM D compiler (1.37.0):
  based on DMD v2.107.1 and LLVM 17.0.6
  built with LDC - the LLVM D compiler (1.37.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.37.0/x64/ldc2-1.37.0-linux-x86_64/bin/ldc2 for x86_64.
    Building dub 1.37.0-rc.1+commit.23.g48d6ecd4: building configuration [application]
     Linking dub
STAT:statistics (-before, +after)
STAT:executable size=5364248 bin/dub
STAT:rough build time=64s

@dlang-bot dlang-bot merged commit 89252c8 into dlang:master Apr 28, 2024
Geod24
Geod24 reviewed Apr 28, 2024
View reviewed changes
source/dub/data/settings.d
standard, /// Does not use the default package suppliers (`defaultPackageSuppliers`).
configured, /// Does not use default suppliers or suppliers configured in DUB's configuration file
all, /// Uses only manually specified package suppliers.
default_, /// The value wasn't specified. It is provided in order to know when it is safe to ignore it
Copy link
Member

@Geod24 Geod24 Apr 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of modifying the enum, passing a Nullable would be a better option, wouldn't it ?

Copy link
Contributor Author

@the-horo the-horo Apr 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I wrote this I considered both passing a Nullable in the constructor or modifying the enum as possible solutions. My final conclusion was that I would rather not change the constructor semantics and, instead, modify the enum.

I wouldn't mind doing as you say and changing the approach to use a Nullable.

the-horo added a commit to the-horo/dub that referenced this pull request May 22, 2025
@the-horo
Replace SkipPackageSuppliers.default_ usage with Nullable
4977ae3 
Instead of having a different enum value for stating that the value is
not specified and it should be taken from the configuration files, use
a Nullable. This has been suggested in
dlang#2903 (comment)

The main reason for this change is going through the other places
where the configuration files were not honored (e.g. dub lint) (which
was the intent of the original PR). It would be a shorter change to
modify the current code to use the .default_ enum value when it was
appropriate but if I have to change the code I might as well go and
implement the less error-prone solution with Nullable.

Signed-off-by: Andrei Horodniceanu <[email protected]>
the-horo added a commit to the-horo/dub that referenced this pull request May 22, 2025
@the-horo
Replace SkipPackageSuppliers.default_ usage with Nullable
06c165a 
Instead of having a different enum value for stating that the value is
not specified and it should be taken from the configuration files, use
a Nullable. This has been suggested in
dlang#2903 (comment)

The main reason for this change is going through the other places
where the configuration files were not honored (e.g. dub lint) (which
was the intent of the original PR). It would be a shorter change to
modify the current code to use the .default_ enum value when it was
appropriate but if I have to change the code I might as well go and
implement the less error-prone solution with Nullable.

Signed-off-by: Andrei Horodniceanu <[email protected]>
the-horo added a commit to the-horo/dub that referenced this pull request May 22, 2025
@the-horo
Replace SkipPackageSuppliers.default_ usage with Nullable
9b48cee 
Instead of having a different enum value for stating that the value is
not specified and it should be taken from the configuration files, use
a Nullable. This has been suggested in
dlang#2903 (comment)

The main reason for this change is going through the other places
where the configuration files were not honored (e.g. dub lint) (which
was the intent of the original PR). It would be a shorter change to
modify the current code to use the .default_ enum value when it was
appropriate but if I have to change the code I might as well go and
implement the less error-prone solution with Nullable.

Signed-off-by: Andrei Horodniceanu <[email protected]>
the-horo added a commit to the-horo/dub that referenced this pull request May 22, 2025
@the-horo
Replace SkipPackageSuppliers.default_ usage with Nullable
06af633 
Instead of having a different enum value for stating that the value is
not specified and it should be taken from the configuration files, use
a Nullable. This has been suggested in
dlang#2903 (comment)

The main reason for this change is going through the other places
where the configuration files were not honored (e.g. dub lint) (which
was the intent of the original PR). It would be a shorter change to
modify the current code to use the .default_ enum value when it was
appropriate but if I have to change the code I might as well go and
implement the less error-prone solution with Nullable.

Signed-off-by: Andrei Horodniceanu <[email protected]>
@the-horo the-horo mentioned this pull request May 22, 2025
Merged
Geod24 pushed a commit that referenced this pull request May 25, 2025
@the-horo @Geod24
Replace SkipPackageSuppliers.default_ usage with Nullable
13aeb7a 
Instead of having a different enum value for stating that the value is
not specified and it should be taken from the configuration files, use
a Nullable. This has been suggested in
#2903 (comment)

The main reason for this change is going through the other places
where the configuration files were not honored (e.g. dub lint) (which
was the intent of the original PR). It would be a shorter change to
modify the current code to use the .default_ enum value when it was
appropriate but if I have to change the code I might as well go and
implement the less error-prone solution with Nullable.

Signed-off-by: Andrei Horodniceanu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Geod24 Geod24 Geod24 left review comments

@thewilsonator thewilsonator thewilsonator approved these changes

Assignees

No one assigned

Labels

auto-merge

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@the-horo @Geod24 @thewilsonator @dlang-bot