Make bash module install into bin/<project>.bash/ to avoid clashes

Also, expect the first component in the import path to be full project
name even in case it has the .bash extension.

6 files changed, 74 insertions, 34 deletions

diff --git a/doc/manual.cli b/doc/manual.cli
index 4fb8f5a..2f532e4 100644
--- a/doc/manual.cli
+++ b/doc/manual.cli
@@ -8754,11 +8754,12 @@ buildfiles.
 
 The \c{say-hello.bash} module is \i{imported} by the \c{hello} script with the
 \c{@import\ hello/say-hello@} substitution. The \i{import path}
-(\c{hello/say-hello} in our case) is a relative path to the module file within
-the project. Its first component (\c{hello} in our case) must be the project
-base name and the \c{.bash} module extension can be omitted. \N{The constraint
-placed on the first component of the import path is required to implement
-importation of installed modules, as discussed below.}
+(\c{hello/say-hello} in our case) is a path to the module file within the
+project. Its first component (\c{hello} in our case) must be both the project
+name and the top-level subdirectory within the project. The \c{.bash} module
+extension can be omitted. \N{The constraint placed on the first component of
+the import path is required to implement importation of installed modules, as
+discussed below.}
 
 During preprocessing, the import substitution will be replaced with a
 \c{source} builtin call and the import path resolved to one of the \c{bash{\}}
@@ -8777,11 +8778,12 @@ OS. The script, however, can provide a suitable implementation as a function.
 See the \c{bash} module tests for a sample implementation of such a function.|
 
 By default, \c{bash} modules are installed into a subdirectory of the \c{bin/}
-installation directory named as the project base name. For instance, in the
-above example, the script will be installed as \c{bin/hello} and the module as
-\c{bin/hello/say-hello.bash} with the script sourcing the module relative to
-the \c{bin/} directory. Note that currently it is assumed the script and all
-its modules are installed into the same \c{bin/} directory.
+installation directory named as the project name plus the \c{.bash} extension.
+For instance, in the above example, the script will be installed as
+\c{bin/hello} and the module as \c{bin/hello.bash/say-hello.bash} with the
+script sourcing the module relative to the \c{bin/} directory. Note that
+currently it is assumed the script and all its modules are installed into the
+same \c{bin/} directory.
 
 Naturally, modules can import other modules and modules can be packaged into
 \i{module libraries} and imported using the standard build system import
@@ -8848,8 +8850,9 @@ for example, \c{libhello}. If there is also a native library (that is, one
 written in C/C++) that provides the same functionality (or the \c{bash}
 library is a language binding for said library), then it is customary to add
 the \c{.bash} extension to the \c{bash} library name, for example,
-\c{libhello.bash}. Note that in this case the project base name is
-\c{libhello}.
+\c{libhello.bash}. Note that in this case the top-level subdirectory within
+the project is expected to be called without the \c{bash} extension,
+for example, \c{libhello}.
 
 Modules can be \i{private} or \i{public}. Private modules are implementation
 details of a specific project and are not expected to be imported from other
diff --git a/libbuild2/bash/init.cxx b/libbuild2/bash/init.cxx
index cf5307f..a1effa1 100644
--- a/libbuild2/bash/init.cxx
+++ b/libbuild2/bash/init.cxx
@@ -48,14 +48,13 @@ namespace build2
       {
         using namespace install;
 
-        // Install into bin/<project>/ by default stripping the .bash
-        // extension from <project> if present.
+        // Install bash{} into bin/<project>.bash/ by default.
         //
         const project_name& p (project (rs));
 
         if (!p.empty ())
         {
-          install_path<bash> (bs, dir_path ("bin") /= project_base (p));
+          install_path<bash> (bs, dir_path ("bin") /= modules_install_dir (p));
           install_mode<bash> (bs, "644");
         }
       }
diff --git a/libbuild2/bash/rule.cxx b/libbuild2/bash/rule.cxx
index f2c1eae..5ba298c 100644
--- a/libbuild2/bash/rule.cxx
+++ b/libbuild2/bash/rule.cxx
@@ -126,7 +126,7 @@ namespace build2
         // apply).
         //
         string ext (p.ext ? *p.ext : "bash");
-        path ip (dir_path (project_base (*p.proj)) / p.dir / p.name);
+        path ip (dir_path (modules_install_dir (*p.proj)) / p.dir / p.name);
 
         if (!ext.empty ())
         {
@@ -217,21 +217,45 @@ namespace build2
                        const target& t,
                        const string& n) const
     {
-      // Derive (relative) import path from the import name.
+      // Derive (relative) import path from the import name. And derive import
+      // installed path from that by adding the .bash extension to the first
+      // component.
       //
-      path ip;
+      path ip, iip;
+      project_name pn;
 
       try
       {
         ip = path (n);
 
-        if (ip.empty () || ip.absolute ())
+        if (ip.empty () || ip.simple () || ip.absolute ())
           throw invalid_path (n);
 
         if (ip.extension_cstring () == nullptr)
           ip += ".bash";
 
         ip.normalize ();
+
+        auto b (ip.begin ()), e (ip.end ());
+
+        try
+        {
+          pn = project_name (*b);
+        }
+        catch (const invalid_argument& e)
+        {
+          fail (l) << "invalid import path '" << n << "': " << e.what ();
+        }
+
+        char s (b++.separator ());
+
+        iip = path (modules_install_dir (pn) + s) / path (b, e);
+
+        // Strip the .bash extension from the project name in this path to
+        // be able to compare it to paths inside the project (see below).
+        //
+        if (pn.extension () == "bash")
+          ip = path (pn.base ("bash") + s) / path (b, e);
       }
       catch (const invalid_path&)
       {
@@ -261,19 +285,19 @@ namespace build2
           //
           // But we still do a simple match first since it can quickly weed
           // out candidates that cannot possibly match.
-          //
-          if (!pp.sup (ip))
-            continue;
 
-          // See if this is import-installed target (refer to search() for
-          // details).
+          // See if this is import-installed target (refer to search() above
+          // for details).
           //
           if (size_t n = pt.data)
           {
+            if (!pp.sup (iip))
+              continue;
+
             // Both are normalized so we can compare the "tails".
             //
             const string& ps (pp.string ());
-            const string& is (ip.string ());
+            const string& is (iip.string ());
 
             if (path::traits_type::compare (
                   ps.c_str () + ps.size () - n, n,
@@ -288,6 +312,9 @@ namespace build2
 
           if (const scope* rs = b->base_scope ().root_scope ())
           {
+            if (!pp.sup (ip) || project (*rs) != pn)
+              continue;
+
             const dir_path& d (pp.sub (rs->src_path ())
                                ? rs->src_path ()
                                : rs->out_path ());
@@ -366,7 +393,7 @@ namespace build2
             "source \"$(dirname"
             " \"$(readlink -f"
             " \"${BASH_SOURCE[0]}\")\")/"
-            + ip.string () + "\"";
+            + iip.string () + "\"";
         }
         else
         {
@@ -387,7 +414,7 @@ namespace build2
           return
             "source \"$(dirname"
             " \"${BASH_SOURCE[0]}\")/"
-            + o + ip.string () + "\"";
+            + o + iip.string () + "\"";
         }
       }
       else
diff --git a/libbuild2/bash/utility.hxx b/libbuild2/bash/utility.hxx
index 087fc38..e5e4377 100644
--- a/libbuild2/bash/utility.hxx
+++ b/libbuild2/bash/utility.hxx
@@ -11,14 +11,26 @@ namespace build2
 {
   namespace bash
   {
-    // Strip the .bash extension from the project name.
+    // Return the bash{} modules installation directory under bin/.
     //
-    // Note that the result may not be a valid project name.
+    // Note that we used to install into bin/<project>/ but that has a good
+    // chance of clashing with the project's executable. Adding the .bash
+    // extension feels like a good idea since in our model the executables
+    // should not use the .bash extension (only modules) and therefore are
+    // unlikely to clash with this name.
+    //
+    // One drawback of this approach is that in case of a project like
+    // libbutl.bash we now have different module directories inside the
+    // project (libbutl/) and when installed (libbutl.bash/). Also, the
+    // installation directory will be shared with the libbutl project but
+    // that's probably ok (and we had the same issue before).
     //
     inline string
-    project_base (const project_name& pn)
+    modules_install_dir (const project_name& pn)
     {
-      return pn.base ("bash");
+      // Strip the .bash extension if present not to duplicate it.
+      //
+      return pn.base ("bash") + ".bash";
     }
   }
 }
diff --git a/libbuild2/install/rule.cxx b/libbuild2/install/rule.cxx
index d4c70c0..b8d716d 100644
--- a/libbuild2/install/rule.cxx
+++ b/libbuild2/install/rule.cxx
@@ -1323,7 +1323,6 @@ namespace build2
         }
       }
 
-
       // Finally handle installable prerequisites.
       //
       r |= reverse_execute_prerequisites (a, t);
diff --git a/tests/bash/testscript b/tests/bash/testscript
index 1e5665c..05f48a1 100644
--- a/tests/bash/testscript
+++ b/tests/bash/testscript
@@ -119,7 +119,7 @@ if ($test.target == $build.host && $build.host.class != 'windows')
         }
         fi
 
-        @import sub/foo@
+        @import sub.bash/foo@
         EOI
 
       cat <<EOI >=buildfile;
@@ -140,7 +140,7 @@ if ($test.target == $build.host && $build.host.class != 'windows')
     :
     {
       cat <<EOI >=test.bash.in;
-        @import sub/foo@
+        @import sub.bash/foo@
         EOI
 
       cat <<EOI >=driver.in;