diff --git a/source/dub/commandline.d b/source/dub/commandline.d index e2baa3b..f877cac 100644 --- a/source/dub/commandline.d +++ b/source/dub/commandline.d @@ -286,7 +286,7 @@ // make the CWD package available so that for example sub packages can reference their // parent package. - try dub.packageManager.getOrLoadPackage(NativePath(options.root_path)); + try dub.packageManager.getOrLoadPackage(NativePath(options.root_path), NativePath.init, false, StrictMode.Warn); catch (Exception e) { logDiagnostic("No valid package found in current working directory: %s", e.msg); } return dub; diff --git a/source/dub/internal/utils.d b/source/dub/internal/utils.d index c4e95c6..89cd65d 100644 --- a/source/dub/internal/utils.d +++ b/source/dub/internal/utils.d @@ -20,6 +20,7 @@ import std.conv : to; import std.exception : enforce; import std.file; +import std.format; import std.string : format; import std.process; import std.traits : isIntegral; @@ -689,3 +690,53 @@ logDiagnostic("Get module name from path: %s", filePath); return getModuleNameFromContent(fileContent); } + +/** + * Compare two instances of the same type for equality, + * providing a rich error message on failure. + * + * This function will recurse into composite types (struct, AA, arrays) + * and compare element / member wise, taking opEquals into account, + * to provide the most accurate reason why comparison failed. + */ +void deepCompare (T) ( + in T result, in T expected, string file = __FILE__, size_t line = __LINE__) +{ + deepCompareImpl!T(result, expected, T.stringof, file, line); +} + +void deepCompareImpl (T) ( + in T result, in T expected, string path, string file, size_t line) +{ + static if (is(T == struct) && !is(typeof(T.init.opEquals(T.init)) : bool)) + { + static foreach (idx; 0 .. T.tupleof.length) + deepCompareImpl(result.tupleof[idx], expected.tupleof[idx], + format("%s.%s", path, __traits(identifier, T.tupleof[idx])), + file, line); + } + else static if (is(T : KeyT[ValueT], KeyT, ValueT)) + { + if (result.length != expected.length) + throw new Exception( + format("%s: AA has different number of entries (%s != %s): %s != %s", + path, result.length, expected.length, result, expected), + file, line); + foreach (key, value; expected) + { + if (auto ptr = key in result) + deepCompareImpl(*ptr, value, format("%s[%s]", path, key), file, line); + else + throw new Exception( + format("Expected key %s[%s] not present in result. %s != %s", + path, key, result, expected), file, line); + } + } + else if (result != expected) { + static if (is(T == struct) && is(typeof(T.init.opEquals(T.init)) : bool)) + path ~= ".opEquals"; + throw new Exception( + format("%s: result != expected: %s != %s", path, result, expected), + file, line); + } +} diff --git a/source/dub/package_.d b/source/dub/package_.d index 8ff61eb..ce7d5a4 100644 --- a/source/dub/package_.d +++ b/source/dub/package_.d @@ -21,6 +21,8 @@ import dub.internal.vibecompat.data.json; import dub.internal.vibecompat.inet.path; +import configy.Read : StrictMode; + import std.algorithm; import std.array; import std.conv; @@ -169,8 +171,11 @@ version_override = Optional version to associate to the package instead of the one declared in the package recipe, or the one determined by invoking the VCS (GIT currently). + mode = Whether to issue errors, warning, or ignore unknown keys in dub.json */ - static Package load(NativePath root, NativePath recipe_file = NativePath.init, Package parent = null, string version_override = "") + static Package load(NativePath root, NativePath recipe_file = NativePath.init, + Package parent = null, string version_override = "", + StrictMode mode = StrictMode.Ignore) { import dub.recipe.io; @@ -181,7 +186,7 @@ .format(root.toNativeString(), packageInfoFiles.map!(f => cast(string)f.filename).join("/"))); - auto recipe = readPackageRecipe(recipe_file, parent ? parent.name : null); + auto recipe = readPackageRecipe(recipe_file, parent ? parent.name : null, mode); auto ret = new Package(recipe, root, parent, version_override); ret.m_infoFile = recipe_file; diff --git a/source/dub/packagemanager.d b/source/dub/packagemanager.d index 6c08bdf..7f85d31 100644 --- a/source/dub/packagemanager.d +++ b/source/dub/packagemanager.d @@ -14,6 +14,9 @@ import dub.internal.vibecompat.inet.path; import dub.internal.logging; import dub.package_; +import dub.recipe.io; +import configy.Exceptions; +public import configy.Read : StrictMode; import std.algorithm : countUntil, filter, sort, canFind, remove; import std.array; @@ -254,17 +257,19 @@ path = NativePath to the root directory of the package recipe_path = Optional path to the recipe file of the package allow_sub_packages = Also return a sub package if it resides in the given folder + mode = Whether to issue errors, warning, or ignore unknown keys in dub.json Returns: The packages loaded from the given path Throws: Throws an exception if no package can be loaded */ - Package getOrLoadPackage(NativePath path, NativePath recipe_path = NativePath.init, bool allow_sub_packages = false) + Package getOrLoadPackage(NativePath path, NativePath recipe_path = NativePath.init, + bool allow_sub_packages = false, StrictMode mode = StrictMode.Ignore) { path.endsWithSlash = true; foreach (p; getPackageIterator()) if (p.path == path && (!p.parentPackage || (allow_sub_packages && p.parentPackage.path != p.path))) return p; - auto pack = Package.load(path, recipe_path); + auto pack = Package.load(path, recipe_path, null, null, mode); addPackages(m_temporaryPackages, pack); return pack; } @@ -790,6 +795,9 @@ } if (!p) p = Package.load(pack_path, packageFile); addPackages(m_packages, p); + } catch (ConfigException exc) { + // Confiy error message already include the path + logError("Invalid recipe for local package: %S", exc); } catch( Exception e ){ logError("Failed to load package in %s: %s", pack_path, e.msg); logDiagnostic("Full error: %s", e.toString().sanitize()); diff --git a/source/dub/project.d b/source/dub/project.d index 7dc5d7a..42cb67f 100644 --- a/source/dub/project.d +++ b/source/dub/project.d @@ -65,7 +65,7 @@ logWarn("There was no package description found for the application in '%s'.", project_path.toNativeString()); pack = new Package(PackageRecipe.init, project_path); } else { - pack = package_manager.getOrLoadPackage(project_path, packageFile); + pack = package_manager.getOrLoadPackage(project_path, packageFile, false, StrictMode.Warn); } this(package_manager, pack); diff --git a/source/dub/recipe/io.d b/source/dub/recipe/io.d index 94bfc91..67d0199 100644 --- a/source/dub/recipe/io.d +++ b/source/dub/recipe/io.d @@ -8,8 +8,9 @@ module dub.recipe.io; import dub.recipe.packagerecipe; +import dub.internal.logging; import dub.internal.vibecompat.inet.path; - +import configy.Read; /** Reads a package recipe from a file. @@ -18,16 +19,20 @@ Params: filename = NativePath of the package recipe file parent_name = Optional name of the parent package (if this is a sub package) + mode = Whether to issue errors, warning, or ignore unknown keys in dub.json Returns: Returns the package recipe contents Throws: Throws an exception if an I/O or syntax error occurs */ -PackageRecipe readPackageRecipe(string filename, string parent_name = null) +PackageRecipe readPackageRecipe( + string filename, string parent_name = null, StrictMode mode = StrictMode.Ignore) { - return readPackageRecipe(NativePath(filename), parent_name); + return readPackageRecipe(NativePath(filename), parent_name, mode); } + /// ditto -PackageRecipe readPackageRecipe(NativePath filename, string parent_name = null) +PackageRecipe readPackageRecipe( + NativePath filename, string parent_name = null, StrictMode mode = StrictMode.Ignore) { import dub.internal.utils : stripUTF8Bom; import dub.internal.vibecompat.core.file : openFile, FileMode; @@ -40,7 +45,7 @@ text = stripUTF8Bom(cast(string)f.readAll()); } - return parsePackageRecipe(text, filename.toNativeString(), parent_name); + return parsePackageRecipe(text, filename.toNativeString(), parent_name, null, mode); } /** Parses an in-memory package recipe. @@ -55,12 +60,13 @@ package) default_package_name = Optional default package name (if no package name is found in the recipe this value will be used) + mode = Whether to issue errors, warning, or ignore unknown keys in dub.json Returns: Returns the package recipe contents Throws: Throws an exception if an I/O or syntax error occurs */ PackageRecipe parsePackageRecipe(string contents, string filename, string parent_name = null, - string default_package_name = null) + string default_package_name = null, StrictMode mode = StrictMode.Ignore) { import std.algorithm : endsWith; import dub.compilers.buildsettings : TargetType; @@ -72,7 +78,46 @@ ret.name = default_package_name; - if (filename.endsWith(".json")) parseJson(ret, parseJsonString(contents, filename), parent_name); + if (filename.endsWith(".json")) + { + try { + ret = parseConfigString!PackageRecipe(contents, filename, mode); + fixDependenciesNames(ret.name, ret); + } catch (ConfigException exc) { + logWarn("Your `dub.json` file use non-conventional features that are deprecated"); + logWarn("Please adjust your `dub.json` file as those warnings will turn into errors in dub v1.40.0"); + logWarn("Error was: %s", exc); + // Fallback to JSON parser + ret = PackageRecipe.init; + parseJson(ret, parseJsonString(contents, filename), parent_name); + } catch (Exception exc) { + logWarn("Your `dub.json` file use non-conventional features that are deprecated"); + logWarn("This is most likely due to duplicated keys."); + logWarn("Please adjust your `dub.json` file as those warnings will turn into errors in dub v1.40.0"); + logWarn("Error was: %s", exc); + // Fallback to JSON parser + ret = PackageRecipe.init; + parseJson(ret, parseJsonString(contents, filename), parent_name); + } + // `debug = ConfigFillerDebug` also enables verbose parser output + debug (ConfigFillerDebug) + { + import std.stdio; + + PackageRecipe jsonret; + parseJson(jsonret, parseJsonString(contents, filename), parent_name); + if (ret != jsonret) + { + writeln("Content of JSON and YAML parsing differ for file: ", filename); + writeln("-------------------------------------------------------------------"); + writeln("JSON (excepted): ", jsonret); + writeln("-------------------------------------------------------------------"); + writeln("YAML (actual ): ", ret); + writeln("========================================"); + ret = jsonret; + } + } + } else if (filename.endsWith(".sdl")) parseSDL(ret, contents, parent_name, filename); else assert(false, "readPackageRecipe called with filename with unknown extension: "~filename); @@ -194,3 +239,56 @@ toSDL(recipe).toSDLDocument(dst); else assert(false, "writePackageRecipe called with filename with unknown extension: "~filename); } + +unittest { + import std.format; + import dub.dependency; + import dub.internal.utils : deepCompare; + + static void success (string source, in PackageRecipe expected, size_t line = __LINE__) { + const result = parseConfigString!PackageRecipe(source, "dub.json"); + deepCompare(result, expected, __FILE__, line); + } + + static void error (string source, string expected, size_t line = __LINE__) { + try + { + auto result = parseConfigString!PackageRecipe(source, "dub.json"); + assert(0, + format("[%s:%d] Exception should have been thrown but wasn't: %s", + __FILE__, line, result)); + } + catch (Exception exc) + assert(exc.toString() == expected, + format("[%s:%s] result != expected: '%s' != '%s'", + __FILE__, line, exc.toString(), expected)); + } + + alias YAMLDep = typeof(BuildSettingsTemplate.dependencies[string.init]); + const PackageRecipe expected1 = + { + name: "foo", + buildSettings: { + dependencies: RecipeDependencyAA([ + "repo": YAMLDep(Dependency(Repository( + "git+https://github.com/dlang/dmd", + "09d04945bdbc0cba36f7bb1e19d5bd009d4b0ff2", + ))), + "path": YAMLDep(Dependency(NativePath("/foo/bar/jar/"))), + "version": YAMLDep(Dependency(VersionRange.fromString("~>1.0"))), + "version2": YAMLDep(Dependency(Version("4.2.0"))), + ])}, + }; + success( + `{ "name": "foo", "dependencies": { + "repo": { "repository": "git+https://github.com/dlang/dmd", + "version": "09d04945bdbc0cba36f7bb1e19d5bd009d4b0ff2" }, + "path": { "path": "/foo/bar/jar/" }, + "version": { "version": "~>1.0" }, + "version2": "4.2.0" +}}`, expected1); + + + error(`{ "name": "bar", "dependencies": {"bad": { "repository": "git+https://github.com/dlang/dmd" }}}`, + "dub.json(0:41): dependencies[bad]: Need to provide a commit hash in 'version' field with 'repository' dependency"); +} diff --git a/source/dub/recipe/packagerecipe.d b/source/dub/recipe/packagerecipe.d index c388a89..e2d1ddc 100644 --- a/source/dub/recipe/packagerecipe.d +++ b/source/dub/recipe/packagerecipe.d @@ -138,6 +138,7 @@ * Build settings can also be found in `configurations`. */ @Optional BuildSettingsTemplate buildSettings; + alias buildSettings this; /** * Specifies a list of command line flags usable for controlling @@ -178,6 +179,27 @@ { string path; PackageRecipe recipe; + + /** + * Given a YAML parser, recurses into `recipe` or use `path` + * depending on the node type. + * + * Two formats are supported for `subpackages`: a string format, + * which is just the path to the subpackage, and embedding the + * full subpackage recipe into the parent package recipe. + * + * To support such a dual syntax, Configy requires the use + * of a `fromYAML` method, as it exposes the underlying format. + */ + static SubPackage fromYAML (scope ConfigParser!SubPackage p) + { + import dyaml.node; + + if (p.node.nodeID == NodeID.mapping) + return SubPackage(null, p.parseAs!PackageRecipe); + else + return SubPackage(p.parseAs!string); + } } /// Describes minimal toolchain requirements @@ -185,15 +207,23 @@ { import std.typecons : Tuple, tuple; + // TODO: We can remove `@Optional` once bosagora/configy#30 is resolved, + // currently it fails because `Dependency.opCmp` is not CTFE-able. + /// DUB version requirement + @Optional @converter((scope ConfigParser!Dependency p) => p.node.as!string.parseDependency) Dependency dub = Dependency.any; /// D front-end version requirement + @Optional @converter((scope ConfigParser!Dependency p) => p.node.as!string.parseDMDDependency) Dependency frontend = Dependency.any; /// DMD version requirement + @Optional @converter((scope ConfigParser!Dependency p) => p.node.as!string.parseDMDDependency) Dependency dmd = Dependency.any; /// LDC version requirement + @Optional @converter((scope ConfigParser!Dependency p) => p.node.as!string.parseDependency) Dependency ldc = Dependency.any; /// GDC version requirement + @Optional @converter((scope ConfigParser!Dependency p) => p.node.as!string.parseDependency) Dependency gdc = Dependency.any; /** Get the list of supported compilers. @@ -222,8 +252,20 @@ /// Bundles information about a build configuration. struct ConfigurationInfo { string name; - string[] platforms; - BuildSettingsTemplate buildSettings; + @Optional string[] platforms; + @Optional BuildSettingsTemplate buildSettings; + alias buildSettings this; + + /** + * Equivalent to the default constructor, used by Configy + */ + this(string name, string[] p, BuildSettingsTemplate build_settings) + @safe pure nothrow @nogc + { + this.name = name; + this.platforms = p; + this.buildSettings = build_settings; + } this(string name, BuildSettingsTemplate build_settings) { @@ -260,11 +302,82 @@ /// Convenience alias as most uses just want to deal with the `Dependency` public alias dependency this; + + /** + * Read a `Dependency` and `BuildSettingsTemplate` from the config file + * + * Required to support both short and long form + */ + static RecipeDependency fromYAML (scope ConfigParser!RecipeDependency p) + { + import dyaml.node; + + if (p.node.nodeID == NodeID.scalar) { + auto d = YAMLFormat(p.node.as!string); + return RecipeDependency(d.toDependency()); + } + auto d = p.parseAs!YAMLFormat; + return RecipeDependency(d.toDependency(), d.settings); + } + + /// In-file of a dependency as specified by the user + private struct YAMLFormat + { + @Name("version") @Optional string version_; + @Optional string path; + @Optional string repository; + bool optional; + @Name("default") bool default_; + + @Optional BuildSettingsTemplate settings; + alias settings this; + + /** + * Used by Configy to provide rich error message when parsing. + * + * Exceptions thrown from `validate` methods will be wrapped with field/file + * informations and rethrown from Configy, providing the user + * with the location of the configuration that triggered the error. + */ + public void validate () const + { + enforce(this.optional || !this.default_, + "Setting default to 'true' has no effect if 'optional' is not set"); + enforce(this.version_.length || this.path.length || this.repository.length, + "Need to provide one of the following fields: 'version', 'path', or 'repository'"); + + enforce(!this.path.length || !this.repository.length, + "Cannot provide a 'path' dependency if a repository dependency is used"); + enforce(!this.repository.length || this.version_.length, + "Need to provide a commit hash in 'version' field with 'repository' dependency"); + + // Need to deprecate this as it's fairly common + version (none) { + enforce(!this.path.length || !this.version_.length, + "Cannot provide a 'path' dependency if a 'version' dependency is used"); + } + } + + /// Turns this struct into a `Dependency` + public Dependency toDependency () const + { + auto result = () { + if (this.path.length) + return Dependency(NativePath(this.path)); + if (this.repository.length) + return Dependency(Repository(this.repository, this.version_)); + return Dependency(VersionRange.fromString(this.version_)); + }(); + result.optional = this.optional; + result.default_ = this.default_; + return result; + } + } } /// Type used to avoid a breaking change when `Dependency[string]` /// was changed to `RecipeDependency[string]` -private struct RecipeDependencyAA +package struct RecipeDependencyAA { /// The underlying data, `public` as `alias this` to `private` field doesn't /// always work. @@ -286,51 +399,60 @@ { this.data[key] = RecipeDependency(dep); } + + /// Configy doesn't like `alias this` to an AA + static RecipeDependencyAA fromYAML (scope ConfigParser!RecipeDependencyAA p) + { + return RecipeDependencyAA(p.parseAs!(typeof(this.data))); + } } /// This keeps general information about how to build a package. /// It contains functions to create a specific BuildSetting, targeted at /// a certain BuildPlatform. struct BuildSettingsTemplate { - RecipeDependencyAA dependencies; - string systemDependencies; - TargetType targetType = TargetType.autodetect; - string targetPath; - string targetName; - string workingDirectory; - string mainSourceFile; - string[string] subConfigurations; - string[][string] dflags; - string[][string] lflags; - string[][string] libs; - string[][string] sourceFiles; - string[][string] sourcePaths; - string[][string] excludedSourceFiles; - string[][string] injectSourceFiles; - string[][string] copyFiles; - string[][string] extraDependencyFiles; - string[][string] versions; - string[][string] debugVersions; - string[][string] versionFilters; - string[][string] debugVersionFilters; - string[][string] importPaths; - string[][string] stringImportPaths; - string[][string] preGenerateCommands; - string[][string] postGenerateCommands; - string[][string] preBuildCommands; - string[][string] postBuildCommands; - string[][string] preRunCommands; - string[][string] postRunCommands; - string[string][string] environments; - string[string][string] buildEnvironments; - string[string][string] runEnvironments; - string[string][string] preGenerateEnvironments; - string[string][string] postGenerateEnvironments; - string[string][string] preBuildEnvironments; - string[string][string] postBuildEnvironments; - string[string][string] preRunEnvironments; - string[string][string] postRunEnvironments; + @Optional RecipeDependencyAA dependencies; + @Optional string systemDependencies; + @Optional TargetType targetType = TargetType.autodetect; + @Optional string targetPath; + @Optional string targetName; + @Optional string workingDirectory; + @Optional string mainSourceFile; + @Optional string[string] subConfigurations; + @StartsWith("dflags") string[][string] dflags; + @StartsWith("lflags") string[][string] lflags; + @StartsWith("libs") string[][string] libs; + @StartsWith("sourceFiles") string[][string] sourceFiles; + @StartsWith("sourcePaths") string[][string] sourcePaths; + @StartsWith("excludedSourceFiles") string[][string] excludedSourceFiles; + @StartsWith("injectSourceFiles") string[][string] injectSourceFiles; + @StartsWith("copyFiles") string[][string] copyFiles; + @StartsWith("extraDependencyFiles") string[][string] extraDependencyFiles; + @StartsWith("versions") string[][string] versions; + @StartsWith("debugVersions") string[][string] debugVersions; + @StartsWith("versionFilters") string[][string] versionFilters; + @StartsWith("debugVersionFilters") string[][string] debugVersionFilters; + @StartsWith("importPaths") string[][string] importPaths; + @StartsWith("stringImportPaths") string[][string] stringImportPaths; + @StartsWith("preGenerateCommands") string[][string] preGenerateCommands; + @StartsWith("postGenerateCommands") string[][string] postGenerateCommands; + @StartsWith("preBuildCommands") string[][string] preBuildCommands; + @StartsWith("postBuildCommands") string[][string] postBuildCommands; + @StartsWith("preRunCommands") string[][string] preRunCommands; + @StartsWith("postRunCommands") string[][string] postRunCommands; + @StartsWith("environments") string[string][string] environments; + @StartsWith("buildEnvironments")string[string][string] buildEnvironments; + @StartsWith("runEnvironments") string[string][string] runEnvironments; + @StartsWith("preGenerateEnvironments") string[string][string] preGenerateEnvironments; + @StartsWith("postGenerateEnvironments") string[string][string] postGenerateEnvironments; + @StartsWith("preBuildEnvironments") string[string][string] preBuildEnvironments; + @StartsWith("postBuildEnvironments") string[string][string] postBuildEnvironments; + @StartsWith("preRunEnvironments") string[string][string] preRunEnvironments; + @StartsWith("postRunEnvironments") string[string][string] postRunEnvironments; + + @StartsWith("buildRequirements") @Optional Flags!BuildRequirement[string] buildRequirements; + @StartsWith("buildOptions") @Optional Flags!BuildOption[string] buildOptions; @@ -630,3 +752,26 @@ assert(bs.sourceFiles == ["src\\foo.d"]); }} } + +/** + * Edit all dependency names from `:foo` to `name:foo`. + * + * TODO: Remove the special case in the parser and remove this hack. + */ +package void fixDependenciesNames (T) (string root, ref T aggr) nothrow +{ + static foreach (idx, FieldRef; T.tupleof) { + static if (is(immutable typeof(FieldRef) == immutable RecipeDependencyAA)) { + string[] toReplace; + foreach (key; aggr.tupleof[idx].byKey) + if (key.length && key[0] == ':') + toReplace ~= key; + foreach (k; toReplace) { + aggr.tupleof[idx][root ~ k] = aggr.tupleof[idx][k]; + aggr.tupleof[idx].data.remove(k); + } + } + else static if (is(typeof(FieldRef) == struct)) + fixDependenciesNames(root, aggr.tupleof[idx]); + } +}