Exclude importPaths that start with ".."

[CyberShadow]

One use case for requiring such a declaration is when the module root is outside the Dub project root. For example, the library "foo" containing a D module "foo.bar" may be in a directory called "foo" and have "bar.d" at the directory root (same directory as "dub.sdl").

Currently, setting importPaths to ".." has the unpleasant side effect of scanning the entire parent directory (which may host other, unrelated projects). Dub does this for reasons such as caching; the build process itself does not require a list of all files that may or may not be imported, as the compiler discovers them not by globbing, but by path construction and file existence checks, based on the names of imported modules and the list of import paths specified with -I.

Ideally, Dub should NEVER recursively glob the importPaths list, and instead communicate with the compiler to discover the full list of files that were actually imported (e.g. from compilers' verbose output). However, this change (which should not affect canonical use cases) should facilitate this particular directory structure.

What do you think?

[github-actions[bot]]

✅ PR OK, no changes in deprecations or warnings

Total deprecations: 15

Total warnings: 0

Build statistics:

 statistics (-before, +after)
-executable size=5276640 bin/dub
-rough build time=75s
+executable size=5272544 bin/dub
+rough build time=76s

Full build output

DUB version 1.31.1, built on Apr 17 2023
LDC - the LLVM D compiler (1.32.1):
  based on DMD v2.102.2 and LLVM 15.0.7
  built with LDC - the LLVM D compiler (1.32.1)
  Default target: x86_64-unknown-linux-gnu
  Host CPU: skylake-avx512
  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
    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
    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.32.1/x64/ldc2-1.32.1-linux-x86_64/bin/ldc2 for x86_64.
    Building dub 1.33.0-beta.1+commit.5.gc4fa5c53: building configuration [application]
source/dub/internal/dyaml/node.d(2513,9): Deprecation: scope variable `this` assigned to non-scope parameter `_param_0` calling `match`
Serializing composite type Flags!(BuildRequirement) which has no serializable fields
Serializing composite type Flags!(BuildOption) which has no serializable fields
source/dub/dependency.d(917,18): Deprecation: scope variable `this` assigned to non-scope parameter `oth` calling `opEquals`
source/dub/dependency.d(920,30): Deprecation: scope variable `this` assigned to non-scope parameter `a` calling `doCmp`
source/dub/dependency.d(921,27): Deprecation: scope variable `this` assigned to non-scope parameter `b` calling `doCmp`
source/dub/dependency.d(939,26): Deprecation: scope variable `this` assigned to non-scope parameter `oth` calling `opEquals`
source/dub/internal/configy/Exceptions.d(130,34): Deprecation: reference to local variable `buffer` assigned to non-scope anonymous parameter
source/dub/internal/configy/Exceptions.d(134,34): Deprecation: reference to local variable `buffer` assigned to non-scope anonymous parameter
source/dub/internal/configy/Exceptions.d(248,27): Deprecation: `@safe` function `formatMessage` calling `formattedWrite`
/opt/hostedtoolcache/dc/ldc2-1.32.1/x64/ldc2-1.32.1-linux-x86_64/bin/../import/std/format/write.d(537,34):        which calls `std.format.spec.FormatSpec!char.FormatSpec.writeUpToNextSpec!(void delegate(in char[]) @safe).writeUpToNextSpec`
/opt/hostedtoolcache/dc/ldc2-1.32.1/x64/ldc2-1.32.1-linux-x86_64/bin/../import/std/format/spec.d(258,33):        which would be `@system` because of:
/opt/hostedtoolcache/dc/ldc2-1.32.1/x64/ldc2-1.32.1-linux-x86_64/bin/../import/std/format/spec.d(258,33):        scope variable `this` assigned to non-scope parameter `e` calling `put`
source/dub/internal/configy/Exceptions.d(250,27): Deprecation: `@safe` function `formatMessage` calling `formattedWrite`
/opt/hostedtoolcache/dc/ldc2-1.32.1/x64/ldc2-1.32.1-linux-x86_64/bin/../import/std/format/write.d(537,34):        which calls `std.format.spec.FormatSpec!char.FormatSpec.writeUpToNextSpec!(void delegate(in char[]) @safe).writeUpToNextSpec`
/opt/hostedtoolcache/dc/ldc2-1.32.1/x64/ldc2-1.32.1-linux-x86_64/bin/../import/std/format/spec.d(258,33):        which would be `@system` because of:
/opt/hostedtoolcache/dc/ldc2-1.32.1/x64/ldc2-1.32.1-linux-x86_64/bin/../import/std/format/spec.d(258,33):        scope variable `this` assigned to non-scope parameter `e` calling `put`
source/dub/internal/configy/Exceptions.d(283,27): Deprecation: `@safe` function `formatMessage` calling `formattedWrite`
/opt/hostedtoolcache/dc/ldc2-1.32.1/x64/ldc2-1.32.1-linux-x86_64/bin/../import/std/format/write.d(537,34):        which calls `std.format.spec.FormatSpec!char.FormatSpec.writeUpToNextSpec!(void delegate(in char[]) @safe).writeUpToNextSpec`
/opt/hostedtoolcache/dc/ldc2-1.32.1/x64/ldc2-1.32.1-linux-x86_64/bin/../import/std/format/spec.d(258,33):        which would be `@system` because of:
/opt/hostedtoolcache/dc/ldc2-1.32.1/x64/ldc2-1.32.1-linux-x86_64/bin/../import/std/format/spec.d(258,33):        scope variable `this` assigned to non-scope parameter `e` calling `put`
source/dub/internal/configy/Exceptions.d(286,27): Deprecation: `@safe` function `formatMessage` calling `formattedWrite`
/opt/hostedtoolcache/dc/ldc2-1.32.1/x64/ldc2-1.32.1-linux-x86_64/bin/../import/std/format/write.d(537,34):        which calls `std.format.spec.FormatSpec!char.FormatSpec.writeUpToNextSpec!(void delegate(in char[]) @safe).writeUpToNextSpec`
/opt/hostedtoolcache/dc/ldc2-1.32.1/x64/ldc2-1.32.1-linux-x86_64/bin/../import/std/format/spec.d(258,33):        which would be `@system` because of:
/opt/hostedtoolcache/dc/ldc2-1.32.1/x64/ldc2-1.32.1-linux-x86_64/bin/../import/std/format/spec.d(258,33):        scope variable `this` assigned to non-scope parameter `e` calling `put`
source/dub/internal/configy/Exceptions.d(323,31): Deprecation: `@safe` function `formatMessage` calling `formattedWrite`
/opt/hostedtoolcache/dc/ldc2-1.32.1/x64/ldc2-1.32.1-linux-x86_64/bin/../import/std/format/write.d(537,34):        which calls `std.format.spec.FormatSpec!char.FormatSpec.writeUpToNextSpec!(void delegate(in char[]) @safe).writeUpToNextSpec`
/opt/hostedtoolcache/dc/ldc2-1.32.1/x64/ldc2-1.32.1-linux-x86_64/bin/../import/std/format/spec.d(258,33):        which would be `@system` because of:
/opt/hostedtoolcache/dc/ldc2-1.32.1/x64/ldc2-1.32.1-linux-x86_64/bin/../import/std/format/spec.d(258,33):        scope variable `this` assigned to non-scope parameter `e` calling `put`
source/dub/internal/configy/Exceptions.d(325,31): Deprecation: `@safe` function `formatMessage` calling `formattedWrite`
/opt/hostedtoolcache/dc/ldc2-1.32.1/x64/ldc2-1.32.1-linux-x86_64/bin/../import/std/format/write.d(537,34):        which calls `std.format.spec.FormatSpec!char.FormatSpec.writeUpToNextSpec!(void delegate(in char[]) @safe).writeUpToNextSpec`
/opt/hostedtoolcache/dc/ldc2-1.32.1/x64/ldc2-1.32.1-linux-x86_64/bin/../import/std/format/spec.d(258,33):        which would be `@system` because of:
/opt/hostedtoolcache/dc/ldc2-1.32.1/x64/ldc2-1.32.1-linux-x86_64/bin/../import/std/format/spec.d(258,33):        scope variable `this` assigned to non-scope parameter `e` calling `put`
source/dub/internal/configy/Exceptions.d(332,31): Deprecation: `@safe` function `formatMessage` calling `formattedWrite`
/opt/hostedtoolcache/dc/ldc2-1.32.1/x64/ldc2-1.32.1-linux-x86_64/bin/../import/std/format/write.d(537,34):        which calls `std.format.spec.FormatSpec!char.FormatSpec.writeUpToNextSpec!(void delegate(in char[]) @safe).writeUpToNextSpec`
/opt/hostedtoolcache/dc/ldc2-1.32.1/x64/ldc2-1.32.1-linux-x86_64/bin/../import/std/format/spec.d(258,33):        which would be `@system` because of:
/opt/hostedtoolcache/dc/ldc2-1.32.1/x64/ldc2-1.32.1-linux-x86_64/bin/../import/std/format/spec.d(258,33):        scope variable `this` assigned to non-scope parameter `e` calling `put`
source/dub/internal/configy/Exceptions.d(335,31): Deprecation: `@safe` function `formatMessage` calling `formattedWrite`
/opt/hostedtoolcache/dc/ldc2-1.32.1/x64/ldc2-1.32.1-linux-x86_64/bin/../import/std/format/write.d(537,34):        which calls `std.format.spec.FormatSpec!char.FormatSpec.writeUpToNextSpec!(void delegate(in char[]) @safe).writeUpToNextSpec`
/opt/hostedtoolcache/dc/ldc2-1.32.1/x64/ldc2-1.32.1-linux-x86_64/bin/../import/std/format/spec.d(258,33):        which would be `@system` because of:
/opt/hostedtoolcache/dc/ldc2-1.32.1/x64/ldc2-1.32.1-linux-x86_64/bin/../import/std/format/spec.d(258,33):        scope variable `this` assigned to non-scope parameter `e` calling `put`
     Linking dub
STAT:statistics (-before, +after)
STAT:executable size=5272544 bin/dub
STAT:rough build time=76s

[WebFreak001]

can you add a test for this?

[CyberShadow]

That's tricky, as the thing to test here is that Dub does not try to read this unrelated file / enter this unrelated directory. It does gracefully skip broken symlinks, so aside something like a logging / error-injecting FUSE filesystem I can't think of anything right now.

[CyberShadow]

If I do chmod 000 on a directory, that does cause current Dub to fail, but I don't think that would make for a very good test. For one, it's POSIX-only; for two, that should probably be fixed to skip unreadable directories anyway, in the same way that broken symlinks are.

[CyberShadow]

Added it as I can't think of anything better right now, and I see the tests are POSIX-only anyway.

[WebFreak001]

you still need to put in the .no_build, .no_test, .no_run files so the CI tests don't try to do those.

[CyberShadow]

BTW I'm seeing zero documentation on how to run the test suite. A section in the README, or a test/README.md, would be nice.

[WebFreak001]

here are some projects which rely on .. in globbing, to test if they still work properly:

for source paths:

• https://github.com/andrey-zherikov/argparse/blob/6fca01c219828b8223b64e1c7899e1cd7c2b2eb9/examples/completion/single_main/app/dub.json#L5

for source files:

• https://github.com/dokutoku/dxlib-d/blob/2e539d4dfda5804fed1a4a0c29957e7dd96f7819/examples/windows/example/dub.json#L20

for import paths:

• https://github.com/dlang-community/gitcompatibledubpackage/blob/master/dub.sdl#L22-L25

it seems that this is even intended documented behavior, although I don't get what it has to do with git submodules.

I think it would have made more sense that importPaths ".." would only scan its own directory, since it's used to make the DUB package name being used as module package name for all files within (and no source directory needed)

[WebFreak001]

I think what we should do for import paths: (and C import paths, since they are the same thing for some compilers)

• make ".." the only allowed import path that can escape the project root (means it will just pass that to the compiler, but only grep within the current project)
  • this requires a deprecation period if users used this for git submodules or things where it would find more files than what is in the package directory
  • this should ensure the folder name is the DUB package name
  • this should not affect regular DUB packages, since they are stored as packagename/version/packagename/{project files}, so the parent folder is known to only contain a single folder with D/C files, with a known and fixed name
• disallow all other relative globs that can escape the project root
• keep absolute globs allowed, for testing purposes, but probably deny them from being published to the registry
  • for users who want to actually use other relative escaping paths, one could use the $PACKAGE_DIR variable to build an absolute path like this

this way we also have proper semantics for importPaths ".." that users can utilize effectively and with solid guarantees.

[CyberShadow]

for source paths:

* https://github.com/andrey-zherikov/argparse/blob/6fca01c219828b8223b64e1c7899e1cd7c2b2eb9/examples/completion/single_main/app/dub.json#L5

Hmm, I think the intent there is to put Dub-related files in their own directory? I'm not sure what to do here. Is it even possible to publish this to the Dub registry? But even if not, importPaths "../source" would be valid and meaningful for a library which did this and then used locally with dub add-local.

for source files:

* https://github.com/dokutoku/dxlib-d/blob/2e539d4dfda5804fed1a4a0c29957e7dd96f7819/examples/windows/example/dub.json#L20

Makes sense.

for import paths:

* https://github.com/dlang-community/gitcompatibledubpackage/blob/master/dub.sdl#L22-L25

Ah, that one's mine! Looks like I made this a long time ago to illustrate the same problem.

I think what we should do for import paths: (and C import paths, since they are the same thing for some compilers)

Sounds like a plan... though, it does bother me that we don't have a solid and defensible mechanism which would be used to decide which sets of paths are filtered, and how.

Perhaps this is the wrong approach entirely, and we need something like a moduleRoot declaration. So then, moduleRoot "foo" would tell Dub that a bar.d in the project root corresponds to the module foo.bar, and that the directory containing the project should be named foo, and that its parent directory should be added to the import (-I) paths; or even use the -mv compiler feature to make things align if the directory structure doesn't allow it.

[WebFreak001]

Perhaps this is the wrong approach entirely, and we need something like a moduleRoot declaration. So then, moduleRoot "foo" would tell Dub that a bar.d in the project root corresponds to the module foo.bar, and that the directory containing the project should be named foo, and that its parent directory should be added to the import (-I) paths; or even use the -mv compiler feature to make things align if the directory structure doesn't allow it.

I think -mv should be avoided by the user, I more see it as something that tools could use to do special purpose stuff, such as sharing the same dependency with multiple versions in a build. Mapping module structure to the file/folder structure makes it easier to reason about what happens and explain to people how things work.

moduleRoot could be one thing how this could be done, but I think keeping "name" for it already makes sense, although it breaks when users use - in their package name.

Do you have a requirement / use-case why you need to control the parent folder/current folder relationship?

[CyberShadow]

I think -mv should be avoided by the user, I more see it as something that tools could use to do special purpose stuff

Yeah sorry, I meant that Dub could use -mv under the hood to implement moduleRoot.

moduleRoot could be one thing how this could be done, but I think keeping "name" for it already makes sense, although it breaks when users use - in their package name.

Well the Dub package name doesn't always correspond to the module name, and special symbols aren't the only case. Probably the most prominent example I'm aware of is arsd_official.

Do you have a requirement / use-case why you need to control the parent folder/current folder relationship?

It's ae and projects structured like it (see that gitcompatibledubpackage project above). The structure allows using both Dub and Git submodules for dependency versioning.

Of course it would be nice to understand and fix the problem as generally as possible. For example, what about something like a package called lib_foo in a directory foo which holds modules in the lib.foo namespace, a package called lib_bar in a directory bar which holds modules in the lib.bar namespace... then, the equivalent importPath would be "../..".