From 183fed527bdbc0cb3e29db1d3dbe1a1002899bc6 Mon Sep 17 00:00:00 2001 From: Zach Smith Date: Sun, 25 Jun 2017 15:52:40 -0400 Subject: [PATCH 1/3] Make conventional structure more explicit in structure warnings and readme --- readme.markdown | 128 +++++++++++++++++++++----------- src/nimblepkg/packageparser.nim | 68 +++++++++-------- tests/tester.nim | 20 ++--- 3 files changed, 134 insertions(+), 82 deletions(-) diff --git a/readme.markdown b/readme.markdown index 8a32227..e1905d6 100644 --- a/readme.markdown +++ b/readme.markdown @@ -29,7 +29,6 @@ Interested in learning **how to create a package**? Skip directly to that sectio - [Tests](#tests) - [Libraries](#libraries) - [Binary packages](#binary-packages) - - [Hybrids](#hybrids) - [Dependencies](#dependencies) - [External dependencies](#external-dependencies) - [Nim compiler](#nim-compiler) @@ -50,14 +49,13 @@ Interested in learning **how to create a package**? Skip directly to that sectio ## Requirements -Nimble has some runtime dependencies on external tools, these tools are -used to download Nimble packages. -For -instance, if a package is hosted on [Github](https://github.com) you require to -have [git](http://www.git-scm.com) installed and added to your environment -``PATH``. Same goes for [Mercurial](http://mercurial.selenic.com) repositories -on [Bitbucket](https://bitbucket.org). Nimble packages are typically hosted in -Git repositories so you may be able to get away without installing Mercurial. +Nimble has some runtime dependencies on external tools, these tools are used to +download Nimble packages. For instance, if a package is hosted on +[Github](https://github.com) you require to have [git](http://www.git-scm.com) +installed and added to your environment ``PATH``. Same goes for +[Mercurial](http://mercurial.selenic.com) repositories on +[Bitbucket](https://bitbucket.org). Nimble packages are typically hosted in Git +repositories so you may be able to get away without installing Mercurial. **Warning:** Ensure that you have a fairly recent version of Git installed. If the version is less recent than 1.9.0 then Nimble may have trouble using it. @@ -188,8 +186,8 @@ Similar to the ``install`` command you can specify a version range, for example: The ``build`` command is mostly used by developers who want to test building their ``.nimble`` package. This command will build the package with default flags, i.e. a debug build which includes stack traces but no GDB debug -information. -The ``install`` command will build the package in release mode instead. +information. The ``install`` command will build the package in release mode +instead. ### nimble c @@ -431,19 +429,54 @@ which are also useful. Take a look at it for more information. ### Project structure -There is nothing surprising about the recommended project structure. The advice -resembles that of many other package managers. +A Nimble project includes a *source directory*, containing a at most one +primary source file, which shares the same name as the project itself (as well +as the project's nimble file). In most cases this source directory will also be +the root directory of the whole project. In all cases, the root directory will +contain the .nimble file. -| Directory | Purpose | -| ------------- | -------------------------------------- | -| ``.`` | Root directory containing .nimble file.| -| ``./src/`` | Project source code | -| ``./tests/`` | Project test files | -| ``./docs/`` | Project documentation | +If the project includes additional source files, or if there is more than one +primary (exported) module, they are all included in a single directory +hierarchy within the source directory. In the case of libraries, this directory +will have the same name as the project (see below for details). -**Note:** Nimble will by default look for source files in ``.``, in order to -use this layout you will need to specify ``srcDir = "src"`` in your .nimble -file. +The source directory can contain additional files and directories +not involved in building the project, as long as they are excluded +in the nimble file. + +Here's a sample one-module project directory: + +``` +. # The root directory of the project, also the source directory +├── LICENSE +├── README.md +├── my_project.nim # The primary source file +├── my_project.nimble # The project nimble file +└── tests # Another source directory, excluded in my_project.nimble + ├── nim.cfg + └── tests.nim +``` + +Here's a more complex example: + +``` +. # Root directory +├── my_project.nimble # Nimble file +├── nim_src # Source directory +│   ├── my_project.nim # Primary source file +│   ├── my_project # Secondary source directory +│   │   ├── util.nim +│   │   ├── common.nim +│   └── tests # Excluded directory +│   ├── nim.cfg +│   └── tests.nim +├── README.rst +└── Makefile +``` + +In this example, the source directory is specified in the .nimble file +with `srcDir = "nim_src"`. Inside of `my_project.nim`, the `util` and `common` +modules will be imported as `my_project.util` and `my_project.common`. #### Tests @@ -485,23 +518,31 @@ into ``$nimbleDir/pkgs/pkgname-ver``. It's up to the package creator to make sur that the package directory layout is correct, this is so that users of the package can correctly import the package. -By convention, it is suggested that the layout be as follows. The directory -layout is determined by the nature of your package, that is, whether your -package exposes only one module or multiple modules. +It is suggested that the layout be as follows. The directory layout is +determined by the nature of your package, that is, whether your package exposes +only one module or multiple modules. If your package exposes only a single module, then that module should be present in the root directory (the directory with the .nimble file) of your git -repository, it is recommended that in this case you name that module whatever -your package's name is. A good example of this is the -[jester](https://github.com/dom96/jester) package which exposes the ``jester`` -module. In this case the jester package is imported with ``import jester``. +repository, and should be named whatever your package's name is. A good example +of this is the [jester](https://github.com/dom96/jester) package which exposes +the ``jester`` module. In this case the jester package is imported with +``import jester``. If your package exposes multiple modules then the modules should be in a ``PackageName`` directory. This will allow for a certain measure of isolation from other packages which expose modules with the same names. In this case the package's modules will be imported with ``import PackageName/module``. -You are free to combine the two approaches described. +Here's a simple example multi-module library package: + +``` +. +├── util +│   ├── useful.nim +│   └── also_useful.nim +└── util.nimble +``` In regards to modules which you do **not** wish to be exposed. You should place them in a ``PackageName/private`` directory. Your modules may then import these @@ -532,26 +573,27 @@ created instead. Other files will be copied in the same way as they are for library packages. -Binary packages should not install .nim files so include -``skipExt = @["nim"]`` in your .nimble file, unless you intend for your package to -be a binary/library combo which is fine. +Binary packages should not install .nim files so include ``skipExt = @["nim"]`` +in your .nimble file, unless you intend for your package to be a binary/library +combo. Dependencies are automatically installed before building. It's a good idea to test that the dependencies you specified are correct by running by running ``nimble build`` or ``nimble install`` in the directory of your package. -### Hybrids +One thing to note about binary packages that contain source files aside from +the one(s) specified in `bin` (or that also expose multiple library modules, as +above) is that a binary may share the name of the package: this will mean +that you will not be able to put your additional .nim files in a ``pkgname`` +directory. The reason for this is that binaries on some operating systems do +not have an extension, so they will clash with a directory of the same name. -One thing to note about library and binary package hybrids is that your binary -may share the name of the package. This will mean that you will -not be able to put your .nim files in a ``pkgname`` directory. The reason you -will not be able to do this is because binaries on some operating systems -do not have an extension so they will clash with a directory of the same name. - -The current -convention to get around this problem is to append ``pkg`` to the name as is -done for nimble. +If this is the case, you should place your additional .nim files in a directory +with `pkg` appended after the name of the project. For instance, if you were +building a binary named `project`, you would put any additional source files in +a directory called `projectpkg`. From within project.nim you would then import +those modules namespaced with `projectpkg/`. ### Dependencies diff --git a/src/nimblepkg/packageparser.nim b/src/nimblepkg/packageparser.nim index f93714d..cbb69dd 100644 --- a/src/nimblepkg/packageparser.nim +++ b/src/nimblepkg/packageparser.nim @@ -69,7 +69,16 @@ proc validatePackageStructure(pkgInfo: PackageInfo, options: Options) = ## This ensures that a package's source code does not leak into ## another package's namespace. ## https://github.com/nim-lang/nimble/issues/144 - let realDir = pkgInfo.getRealDir() + let + realDir = pkgInfo.getRealDir() + normalizedBinNames = pkgInfo.bin.map( + (x) => x.changeFileExt("").toLowerAscii() + ) + correctDir = + if pkgInfo.name.toLowerAscii() in normalizedBinNames: + pkgInfo.name & "pkg" + else: + pkgInfo.name for path in getInstallFiles(realDir, pkgInfo, options): # Remove the root to leave only the package subdirectories. # ~/package-0.1/package/utils.nim -> package/utils.nim. @@ -83,39 +92,40 @@ proc validatePackageStructure(pkgInfo: PackageInfo, options: Options) = if dir.len == 0: if file != pkgInfo.name: - let msg = ("File inside package '$1' is outside of permitted " & - "namespace, should be " & - "named '$2' but was named '$3' instead. This will be an error" & - " in the future.") % - [pkgInfo.name, pkgInfo.name & ext, file & ext] - let hint = ("Rename this file to '$1', move it into a '$2' " & - "subdirectory, or prevent its installation by adding " & - "`skipFiles = @[\"$3\"]` to the .nimble file. See " & - "https://github.com/nim-lang/nimble#libraries for more info.") % - [pkgInfo.name & ext, pkgInfo.name & DirSep, file & ext] + # A source file was found in the top level of srcDir that doesn't share + # a name with the package. + let + msg = ("Package '$1' has an incorrect structure. " & + "The top level of the package source directory " & + "should contain at most one module, " & + "named '$2', but a file named '$3' was found. This " & + "will be an error in the future.") % + [pkgInfo.name, pkgInfo.name & ext, file & ext] + hint = ("If this is the primary source file in the package, " & + "Rename it to '$1'. If it's a source file required by " & + "the main module, or if it is one of several " & + "modules exposed by '$4', then move it into a '$2' subdirectory. " & + "If it's a test file or otherwise not required " & + "to build the the package '$1', prevent its installation " & + "by adding `skipFiles = @[\"$3\"]` to the .nimble file. See " & + "https://github.com/nim-lang/nimble#libraries for more info.") % + [pkgInfo.name & ext, correctDir & DirSep, file & ext, pkgInfo.name] raiseNewValidationError(msg, true, hint, true) else: assert(not pkgInfo.isMinimal) # On Windows `pkgInfo.bin` has a .exe extension, so we need to normalize. - let normalizedBinNames = pkgInfo.bin.map( - (x) => x.changeFileExt("").toLowerAscii() - ) - let correctDir = - if pkgInfo.name.toLowerAscii() in normalizedBinNames: - pkgInfo.name & "pkg" - else: - pkgInfo.name - if not (dir.startsWith(correctDir & DirSep) or dir == correctDir): - let msg = ("File '$1' inside package '$2' is outside of the" & - " permitted namespace" & - ", should be inside a directory named '$3' but is in a" & - " directory named '$4' instead. This will be an error in the " & - "future.") % - [file & ext, pkgInfo.name, correctDir, dir] - let hint = ("Rename the directory to '$1' or prevent its " & - "installation by adding `skipDirs = @[\"$2\"]` to the " & - ".nimble file.") % [correctDir, dir] + let + msg = ("Package '$2' has an incorrect structure. " & + "It should contain a single directory hierarchy " & + "For source files, named '$3', but file '$1' " & + "is in a directory named '$4' instead. " & + "This will be an error in the future.") % + [file & ext, pkgInfo.name, correctDir, dir] + hint = ("If '$1' contains source files for building '$2', rename it " & + "to '$3'. Otherwise, prevent its installation " & + "by adding `skipDirs = @[\"$1\"]` to the .nimble file.") % + [dir, pkgInfo.name, correctDir] raiseNewValidationError(msg, true, hint, true) proc validatePackageInfo(pkgInfo: PackageInfo, options: Options) = diff --git a/tests/tester.nim b/tests/tester.nim index 8cbf3d6..0b3d42c 100644 --- a/tests/tester.nim +++ b/tests/tester.nim @@ -80,19 +80,19 @@ test "can validate package structure (#144)": let lines = output.strip.splitLines() case package of "x": - check inLines(lines, "File 'foobar.nim' inside package 'x' is outside" & - " of the permitted namespace, should be inside a" & - " directory named 'x' but is in a directory named" & + check inLines(lines, "Package 'x' has an incorrect structure. It should" & + " contain a single directory hierarchy for source files," & + " named 'x', but file 'foobar.nim' is in a directory named" & " 'incorrect' instead.") of "y": - check inLines(lines, "File 'foobar.nim' inside package 'y' is outside" & - " of the permitted namespace, should be inside a" & - " directory named 'ypkg' but is in a directory" & - " named 'yWrong' instead.") + check inLines(lines, "Package 'y' has an incorrect structure. It should" & + " contain a single directory hierarchy for source files," & + " named 'ypkg', but file 'foobar.nim' is in a directory named" & + " 'yWrong' instead.") of "z": - check inLines(lines, "File inside package 'z' is outside of permitted" & - " namespace, should be named 'z.nim' but was" & - " named 'incorrect.nim' instead.") + check inLines(lines, "Package 'z' has an incorrect structure. The top level" & + " of the package source directory should contain at most one module," & + " named 'z.nim', but a file named 'incorrect.nim' was found.") else: assert false From 2a7e1f132c70bdef502f792556a44b8e1e28d89d Mon Sep 17 00:00:00 2001 From: Zach Smith Date: Mon, 26 Jun 2017 17:12:46 -0400 Subject: [PATCH 2/3] Code Review from @dom96 --- readme.markdown | 27 ++++----------------------- src/nimblepkg/packageparser.nim | 4 ++-- 2 files changed, 6 insertions(+), 25 deletions(-) diff --git a/readme.markdown b/readme.markdown index e1905d6..e90f0a0 100644 --- a/readme.markdown +++ b/readme.markdown @@ -429,7 +429,7 @@ which are also useful. Take a look at it for more information. ### Project structure -A Nimble project includes a *source directory*, containing a at most one +A Nimble project includes a *source directory*, containing at most one primary source file, which shares the same name as the project itself (as well as the project's nimble file). In most cases this source directory will also be the root directory of the whole project. In all cases, the root directory will @@ -457,27 +457,6 @@ Here's a sample one-module project directory: └── tests.nim ``` -Here's a more complex example: - -``` -. # Root directory -├── my_project.nimble # Nimble file -├── nim_src # Source directory -│   ├── my_project.nim # Primary source file -│   ├── my_project # Secondary source directory -│   │   ├── util.nim -│   │   ├── common.nim -│   └── tests # Excluded directory -│   ├── nim.cfg -│   └── tests.nim -├── README.rst -└── Makefile -``` - -In this example, the source directory is specified in the .nimble file -with `srcDir = "nim_src"`. Inside of `my_project.nim`, the `util` and `common` -modules will be imported as `my_project.util` and `my_project.common`. - #### Tests A common problem that arises with tests is the fact that they need to import @@ -534,7 +513,7 @@ If your package exposes multiple modules then the modules should be in a from other packages which expose modules with the same names. In this case the package's modules will be imported with ``import PackageName/module``. -Here's a simple example multi-module library package: +Here's a simple example multi-module library package called `util`: ``` . @@ -582,6 +561,8 @@ It's a good idea to test that the dependencies you specified are correct by running by running ``nimble build`` or ``nimble install`` in the directory of your package. +### Hybrids + One thing to note about binary packages that contain source files aside from the one(s) specified in `bin` (or that also expose multiple library modules, as above) is that a binary may share the name of the package: this will mean diff --git a/src/nimblepkg/packageparser.nim b/src/nimblepkg/packageparser.nim index cbb69dd..37bb679 100644 --- a/src/nimblepkg/packageparser.nim +++ b/src/nimblepkg/packageparser.nim @@ -102,7 +102,7 @@ proc validatePackageStructure(pkgInfo: PackageInfo, options: Options) = "will be an error in the future.") % [pkgInfo.name, pkgInfo.name & ext, file & ext] hint = ("If this is the primary source file in the package, " & - "Rename it to '$1'. If it's a source file required by " & + "rename it to '$1'. If it's a source file required by " & "the main module, or if it is one of several " & "modules exposed by '$4', then move it into a '$2' subdirectory. " & "If it's a test file or otherwise not required " & @@ -118,7 +118,7 @@ proc validatePackageStructure(pkgInfo: PackageInfo, options: Options) = let msg = ("Package '$2' has an incorrect structure. " & "It should contain a single directory hierarchy " & - "For source files, named '$3', but file '$1' " & + "for source files, named '$3', but file '$1' " & "is in a directory named '$4' instead. " & "This will be an error in the future.") % [file & ext, pkgInfo.name, correctDir, dir] From 34cc9b958f759d0f61bbd510ff131f7ab0c1b0d4 Mon Sep 17 00:00:00 2001 From: Zach Smith Date: Mon, 26 Jun 2017 20:29:36 -0400 Subject: [PATCH 3/3] Restore Hybrids heading to readme --- readme.markdown | 1 + 1 file changed, 1 insertion(+) diff --git a/readme.markdown b/readme.markdown index e90f0a0..7bf8d1d 100644 --- a/readme.markdown +++ b/readme.markdown @@ -29,6 +29,7 @@ Interested in learning **how to create a package**? Skip directly to that sectio - [Tests](#tests) - [Libraries](#libraries) - [Binary packages](#binary-packages) + - [Hybrids](#hybrids) - [Dependencies](#dependencies) - [External dependencies](#external-dependencies) - [Nim compiler](#nim-compiler)