From 9b238b9b81a04554edbd6bc35d12aac0433cd511 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Wed, 26 Jan 2022 13:00:34 +0200 Subject: Make bash module install into bin/.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. --- doc/manual.cli | 27 +++++++++++++----------- libbuild2/bash/init.cxx | 5 ++--- libbuild2/bash/rule.cxx | 51 +++++++++++++++++++++++++++++++++++----------- libbuild2/bash/utility.hxx | 20 ++++++++++++++---- libbuild2/install/rule.cxx | 1 - tests/bash/testscript | 4 ++-- 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// by default stripping the .bash - // extension from if present. + // Install bash{} into bin/.bash/ by default. // const project_name& p (project (rs)); if (!p.empty ()) { - install_path (bs, dir_path ("bin") /= project_base (p)); + install_path (bs, dir_path ("bin") /= modules_install_dir (p)); install_mode (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// 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 <=buildfile; @@ -140,7 +140,7 @@ if ($test.target == $build.host && $build.host.class != 'windows') : { cat <=test.bash.in; - @import sub/foo@ + @import sub.bash/foo@ EOI cat <=driver.in; -- cgit v1.1