Skip to content
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

Respect skipRegistry value from configuration files #2903

Merged
merged 1 commit into from
Apr 28, 2024
Merged

Respect skipRegistry value from configuration files #2903

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 26 additions & 2 deletions source/dub/commandline.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -565,7 +565,7 @@ struct CommonOptions {
string root_path, recipeFile;
enum Color { automatic, on, off }
Color colorMode = Color.automatic;
SkipPackageSuppliers skipRegistry = SkipPackageSuppliers.none;
SkipPackageSuppliers skipRegistry = SkipPackageSuppliers.default_;
PlacementLocation placementLocation = PlacementLocation.user;

deprecated("Use `Color` instead, the previous naming was a limitation of error message formatting")
Expand All @@ -591,6 +591,30 @@ struct CommonOptions {
~ "', supported values: --color[=auto], --color=always, --color=never");
}

private void parseSkipRegistry(string option, string value) @safe
{
// We only want to support `none`, `standard`, `configured`, and `all`.
// We use a separate function to prevent getopt from parsing SkipPackageSuppliers.default_.
assert(option == "skip-registry",
"parseSkipRegistry called with unknown option '" ~ option ~ "'");
switch (value) with (SkipPackageSuppliers) {
case "none":
skipRegistry = none;
break;
case "standard":
skipRegistry = standard;
break;
case "configured":
skipRegistry = configured;
break;
case "all":
skipRegistry = all;
break;
default:
throw new GetOptException("skip-registry only accepts 'none', 'standard', 'configured', and 'all', not '" ~ value ~ "'");
}
}

/// Parses all common options and stores the result in the struct instance.
void prepare(CommandArgs args)
{
Expand All @@ -602,7 +626,7 @@ struct CommonOptions {
" DUB: URL to DUB registry (default)",
" Maven: URL to Maven repository + group id containing dub packages as artifacts. E.g. mvn+http://localhost:8040/maven/libs-release/dubpackages",
]);
args.getopt("skip-registry", &skipRegistry, [
args.getopt("skip-registry", &skipRegistry, &parseSkipRegistry, [
"Sets a mode for skipping the search on certain package registry types:",
" none: Search all configured or default registries (default)",
" standard: Don't search the main registry (e.g. "~defaultRegistryURLs[0]~")",
Expand Down
30 changes: 29 additions & 1 deletion source/dub/data/settings.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ public enum SkipPackageSuppliers {
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

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

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.

}

/**
Expand All @@ -39,7 +40,22 @@ package(dub) struct Settings {
@Optional string[] registryUrls;
@Optional NativePath[] customCachePaths;

SetInfo!(SkipPackageSuppliers) skipRegistry;
private struct SkipRegistry {
SkipPackageSuppliers skipRegistry;
static SkipRegistry fromString (string value) {
import std.conv : to;
auto result = value.to!SkipPackageSuppliers;
if (result == SkipPackageSuppliers.default_) {
throw new Exception(
"skipRegistry value `default_` is only meant for interal use."
~ " Instead, use one of `none`, `standard`, `configured`, or `all`."
);
}
return SkipRegistry(result);
}
alias skipRegistry this;
}
SetInfo!(SkipRegistry) skipRegistry;
SetInfo!(string) defaultCompiler;
SetInfo!(string) defaultArchitecture;
SetInfo!(bool) defaultLowMemory;
Expand Down Expand Up @@ -171,3 +187,15 @@ unittest {
auto m3 = Settings.init.merge(c1);
assert(m3 == c1);
}

unittest {
// Test that SkipPackageRegistry.default_ is not allowed

import dub.internal.configy.Read;
import std.exception : assertThrown;

const str1 = `{
"skipRegistry": "all"
`;
assertThrown!Exception(parseConfigString!Settings(str1, "/dev/null"));
}
7 changes: 7 additions & 0 deletions source/dub/dub.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,13 @@ class Dub {

init();

if (skip == SkipPackageSuppliers.default_) {
// If unspecified on the command line, take
// the value from the configuration files, or
// default to none.
skip = m_config.skipRegistry.set ? m_config.skipRegistry.value : SkipPackageSuppliers.none;
}

const registry_var = environment.get("DUB_REGISTRY", null);
m_packageSuppliers = this.makePackageSuppliers(base, skip, registry_var);
m_packageManager = this.makePackageManager();
Expand Down
Loading