From df91839a0f53b1bf266eb6b9ebabf2b587211731 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Tue, 21 Mar 2023 06:32:56 +0200 Subject: Add support for --recursive=separate pkg-bindist option mode --- bpkg/pkg-bindist.cli | 92 +++++----- bpkg/pkg-bindist.cxx | 314 ++++++++++++++++++++++---------- bpkg/system-package-manager-archive.cxx | 22 +-- bpkg/system-package-manager-archive.hxx | 3 +- bpkg/system-package-manager-debian.cxx | 20 +- bpkg/system-package-manager-debian.hxx | 3 +- bpkg/system-package-manager-fedora.cxx | 7 +- bpkg/system-package-manager-fedora.hxx | 3 +- bpkg/system-package-manager.hxx | 20 +- 9 files changed, 304 insertions(+), 180 deletions(-) diff --git a/bpkg/pkg-bindist.cli b/bpkg/pkg-bindist.cli index db372b8..7116e7d 100644 --- a/bpkg/pkg-bindist.cli +++ b/bpkg/pkg-bindist.cli @@ -79,18 +79,18 @@ namespace bpkg bpkg bindist -o /tmp/output/ libhello \ - Unless the \cb{--recursive} option is specified, dependencies of the - specified package are translated to dependencies in the resulting binary - package using names and versions that refer to packages that would be - generated by the \cb{pkg-bindist} command (called \"non-native\" - packages). If instead you would like certain dependencies to refer to - binary packages provided by the distribution (called \"native\" - packages), then you need to arrange for them to be built as system (see - \l{bpkg-pkg-build(1)} for details). For example, if our \cb{libhello} has - a dependency on \cb{libsqlite3} and we would like the binary package for - \cb{libhello} to refer to \cb{libsqlite3} from Debian (or alike), then - the \cb{pkg-build} command would need to be (\cb{--sys-install} is - optional): + Unless the \cb{--recursive} option \cb{auto} or \cb{full} modes are + specified, dependencies of the specified package are translated to + dependencies in the resulting binary package using names and versions + that refer to packages that would be generated by the \cb{pkg-bindist} + command (called \"non-native\" packages). If instead you would like + certain dependencies to refer to binary packages provided by the + distribution (called \"native\" packages), then you need to arrange for + them to be built as system (see \l{bpkg-pkg-build(1)} for details). For + example, if our \cb{libhello} has a dependency on \cb{libsqlite3} and we + would like the binary package for \cb{libhello} to refer to + \cb{libsqlite3} from Debian (or alike), then the \cb{pkg-build} command + would need to be (\cb{--sys-install} is optional): \ bpkg build --sys-install libhello ?sys:libsqlite3 @@ -245,18 +245,18 @@ namespace bpkg The resulting binary packages are placed into the standard \cb{rpmbuild} output directory (normally \c{\b{~/rpmbuild/RPMS/}\i{arch}\b{/}}). - Unless the \cb{--recursive} option is specified, dependencies of the - specified package are translated to dependencies in the resulting binary - package using names and versions that refer to packages that would be - generated by the \cb{pkg-bindist} command (called \"non-native\" - packages). If instead you would like certain dependencies to refer to - binary packages provided by the distribution (called \"native\" - packages), then you need to arrange for them to be built as system (see - \l{bpkg-pkg-build(1)} for details). For example, if our \cb{libhello} has - a dependency on \cb{libsqlite3} and we would like the binary package for - \cb{libhello} to refer to \cb{sqlite-libs} from Fedora (or alike), then - the \cb{pkg-build} command would need to be (\cb{--sys-install} is - optional): + Unless the \cb{--recursive} option \cb{auto} or \cb{full} modes are + specified, dependencies of the specified package are translated to + dependencies in the resulting binary package using names and versions + that refer to packages that would be generated by the \cb{pkg-bindist} + command (called \"non-native\" packages). If instead you would like + certain dependencies to refer to binary packages provided by the + distribution (called \"native\" packages), then you need to arrange for + them to be built as system (see \l{bpkg-pkg-build(1)} for details). For + example, if our \cb{libhello} has a dependency on \cb{libsqlite3} and we + would like the binary package for \cb{libhello} to refer to + \cb{sqlite-libs} from Fedora (or alike), then the \cb{pkg-build} command + would need to be (\cb{--sys-install} is optional): \ bpkg build --sys-install libhello ?sys:libsqlite3 @@ -615,28 +615,31 @@ namespace bpkg string --recursive { "", - "Bundle dependencies of the specified packages. The value can be - either \cb{auto}, in which case only the required files from each - dependency package are bundled, or \cb{full}, in which case all the - files are bundled. Specifically, in the \cb{auto} mode any required - files, for example, shared libraries, are pulled implicitly by the - \cb{install} build system operation, for example, as part of - installing an executable from one of the specified packages. In - contrast, in the \cb{full} mode, each dependency package is - installed explicitly and completely, as if they were specified - as additional package on the command line. See also the \cb{--private} - option." + "Bundle or generate dependencies of the specified packages. The + value can be either \cb{auto}, in which case only the required files + from each dependency package are bundled, \cb{full}, in which case + all the files are bundled, or \cb{separate}, in which case a separate + binary package is generated for each non-system dependency. + + Specifically, in the \cb{auto} mode any required files, such as shared + libraries, are pulled implicitly by the \cb{install} build system + operation, for example, as part of installing an executable from one of + the specified packages. In contrast, in the \cb{full} mode, each + dependency package is installed explicitly and completely, as if they + were specified as additional package on the command line. The + \cb{separate} mode is equivalent to invoking the \cb{pkg-bindist} + command on each dependency package. See also the \cb{--private} option." } bool --private { "Enable the private installation subdirectory functionality using the - package name as the private subdirectory. This is primarily useful - when bundling dependencies, such as shared libraries, of an executable - that is being installed into a shared location, such as \cb{/usr/}. - See the \cb{config.install.private} configuration variable - documentation in the build system manual for details. This option only - makes sense together with \cb{--recursive}." + package name as the private subdirectory. This is primarily useful when + bundling dependencies, such as shared libraries, of an executable that + is being installed into a shared location, such as \cb{/usr/}. See the + \cb{config.install.private} configuration variable documentation in the + build system manual for details. This option only makes sense together + with the \cb{--recursive} option \cb{auto} and \cb{full} modes." } dir_path --output-root|-o @@ -662,6 +665,13 @@ namespace bpkg troubleshooting." } + bool --allow-dependent-config + { + "Allow configuration that is imposed by dependent packages. Normally + this is undesirable because the resulting binary packages become + configured specificaly for particular dependent packages." + } + string --os-release-id { "", diff --git a/bpkg/pkg-bindist.cxx b/bpkg/pkg-bindist.cxx index 8c2163b..cb29af1 100644 --- a/bpkg/pkg-bindist.cxx +++ b/bpkg/pkg-bindist.cxx @@ -3,6 +3,8 @@ #include +#include + #include #include #include @@ -18,7 +20,6 @@ namespace bpkg { using package = system_package_manager::package; using packages = system_package_manager::packages; - using recursive_mode = system_package_manager::recursive_mode; // Find the available package(s) for the specified selected package. // @@ -220,6 +221,8 @@ namespace bpkg // Verify options. // + enum class recursive_mode {auto_, full, separate}; + optional rec; { diag_record dr; @@ -228,13 +231,24 @@ namespace bpkg { const string& m (o.recursive ()); - if (m == "auto") rec = recursive_mode::auto_; - else if (m == "full") rec = recursive_mode::full; + if (m == "auto") rec = recursive_mode::auto_; + else if (m == "full") rec = recursive_mode::full; + else if (m == "separate") rec = recursive_mode::separate; else dr << fail << "unknown mode '" << m << "' specified with --recursive"; } - else if (o.private_ ()) - dr << fail << "--private specified without --recursive"; + + if (o.private_ ()) + { + if (!rec) + { + dr << fail << "--private specified without --recursive"; + } + else if (*rec == recursive_mode::separate) + { + dr << fail << "--private specified without --recursive=separate"; + } + } if (!dr.empty ()) dr << info << "run 'bpkg help pkg-bindist' for more information"; @@ -281,6 +295,25 @@ namespace bpkg info << "run 'bpkg help pkg-bindist' for more information"; } + // Note that we shouldn't need to install anything or use sudo. + // + unique_ptr spm ( + make_production_system_package_manager (o, + host_triplet, + o.distribution (), + o.architecture ())); + if (spm == nullptr) + { + fail << "no standard distribution package manager for this host " + << "or it is not yet supported" << + info << "consider specifying alternative distribution package " + << "manager with --distribution" << + info << "specify --distribution=archive to generate installation " + << "archive" << + info << "consider specifying --os-release-* if unable to correctly " + << "auto-detect host operating system"; + } + database db (c, trace, true /* pre_attach */); // Similar to pkg-install we disallow generating packages from the @@ -303,128 +336,211 @@ namespace bpkg // session ses; - // Resolve package names to selected packages and verify they are all - // configured. While at it collect their available packages and - // dependencies as well as figure out type and languages. + // Generate one binary package. // - packages pkgs, deps; - string type; - small_vector langs; - - for (const package_name& n: pns) + struct result + { + paths bins; + packages deps; + shared_ptr pkg; + }; + + auto generate = [&o, &vars, + rec, &spm, + &c, &db] (const vector& pns, + bool first) -> result { - shared_ptr p (db.find (n)); + // Resolve package names to selected packages and verify they are all + // configured. While at it collect their available packages and + // dependencies as well as figure out type and languages. + // + packages pkgs, deps; + string type; + small_vector langs; - if (p == nullptr) - fail << "package " << n << " does not exist in configuration " << c; + for (const package_name& n: pns) + { + shared_ptr p (db.find (n)); - if (p->state != package_state::configured) - fail << "package " << n << " is " << p->state << - info << "expected it to be configured"; + if (p == nullptr) + fail << "package " << n << " does not exist in configuration " << c; - if (p->substate == package_substate::system) - fail << "package " << n << " is configured as system"; + if (p->state != package_state::configured) + fail << "package " << n << " is " << p->state << + info << "expected it to be configured"; - // Load the available package for type/languages as well as the mapping - // information. - // - available_packages aps (find_available_packages (o, db, p)); - const shared_ptr& ap (aps.front ().first); - db.load (*ap, ap->languages_section); + if (p->substate == package_substate::system) + fail << "package " << n << " is configured as system"; - if (pkgs.empty ()) // First. - { - type = ap->effective_type (); - langs = ap->effective_languages (); - } - else - merge_languages (type, langs, *ap); + // Make sure there are no dependent configuration variables. The + // rationale here is that we most likely don't want to generate a + // binary package in a configuration that is specific to some + // dependents. + // + if (!o.allow_dependent_config ()) + { + for (const config_variable& v: p->config_variables) + { + switch (v.source) + { + case config_source::dependent: + { + fail << "configuration variable " << v.name << " is imposed " + << " by dependent package" << + info << "specify it as user configuration to allow" << + info << "or specify --allow-dependent-config" << endf; + } + case config_source::user: + case config_source::reflect: + break; + } + } + } + + // Load the available package for type/languages as well as the + // mapping information. + // + available_packages aps (find_available_packages (o, db, p)); + const shared_ptr& ap (aps.front ().first); + db.load (*ap, ap->languages_section); + + if (pkgs.empty ()) // First. + { + type = ap->effective_type (); + langs = ap->effective_languages (); + } + else + merge_languages (type, langs, *ap); - const selected_package& r (*p); - pkgs.push_back ( - package {move (p), move (aps), r.effective_out_root (db.config)}); + const selected_package& r (*p); + pkgs.push_back ( + package {move (p), move (aps), r.effective_out_root (db.config)}); - // If --recursive is not specified then we want all the immediate - // (system and non-) dependecies in deps. Otherwise, if the recursive - // mode is full, then we want all the transitive non-system dependecies - // in pkgs. In both recursive modes we also want all the transitive - // system dependecies in deps. + // If --recursive is not specified or specified with the seperate mode + // then we want all the immediate (system and non-) dependecies in + // deps. Otherwise, if the recursive mode is full, then we want all + // the transitive non-system dependecies in pkgs. In both recursive + // modes we also want all the transitive system dependecies in deps. + // + // Note also that in the auto recursive mode it's possible that some + // of the system dependencies are not really needed. But there is no + // way for us to detect this and it's better to over- than + // under-specify. + // + collect_dependencies ( + o, + db, + (!rec || *rec == recursive_mode::separate + ? &deps + : *rec == recursive_mode::full ? &pkgs : nullptr), + deps, + type, + langs, + r, + rec.has_value ()); + } + + // Load the package manifest (source of extra metadata). This should be + // always possible since the package is configured and is not system. // - // Note also that in the auto recursive mode it's possible that some of - // the system dependencies are not really needed. But there is no way - // for us to detect this and it's better to over- than under-specify. + const shared_ptr& sp (pkgs.front ().selected); + + package_manifest pm ( + pkg_verify (o, + sp->effective_src_root (db.config_orig), + true /* ignore_unknown */, + false /* ignore_toolchain */, + false /* load_buildfiles */, + // Copy potentially fixed up version from selected package. + [&sp] (version& v) {v = sp->version;})); + + optional recursive_full; + if (rec && *rec != recursive_mode::separate) + recursive_full = *rec != recursive_mode::full; + + // Note that we pass type from here in case one day we want to provide + // an option to specify/override it (along with languages). Note that + // there will probably be no way to override type for dependencies. // - collect_dependencies (o, - db, - (rec - ? *rec == recursive_mode::full ? &pkgs : nullptr - : &deps), - deps, - type, - langs, - r, - rec.has_value ()); - } + paths r (spm->generate (pkgs, + deps, + vars, + db.config, + pm, + type, langs, + recursive_full, + first)); + + return result {move (r), move (deps), move (pkgs.front ().selected)}; + }; + + list rs; // Note: list for reference stability. + + // Generate packages for dependencies, recursively, suppressing + // duplicates. Note: recursive lambda. + // + auto generate_deps = [&generate, &rs] (const packages& deps, + const auto& generate_deps) -> void + { + for (const package& d: deps) + { + const shared_ptr& p (d.selected); - t.commit (); + // Skip system dependencies. + // + if (p->substate == package_substate::system) + continue; - // Load the package manifest (source of extra metadata). This should be - // always possible since the package is configured and is not system. - // - const shared_ptr& sp (pkgs.front ().selected); + // Make sure we don't generate the same dependency multiple times. + // + if (find_if (rs.begin (), rs.end (), + [&p] (const result& r) + { + return r.pkg == p; + }) != rs.end ()) + continue; - package_manifest pm ( - pkg_verify (o, - sp->effective_src_root (db.config_orig), - true /* ignore_unknown */, - false /* ignore_toolchain */, - false /* load_buildfiles */, - // Copy potentially fixed up version from selected package. - [&sp] (version& v) {v = sp->version;})); + if (verb >= 1) + text << "generating package for dependency " << p->name; - // Note that we shouldn't need to install anything or use sudo. + rs.push_back (generate ({p->name}, false /* first */)); + generate_deps (rs.back ().deps, generate_deps); + } + }; + + // Generate top-level package(s). // - unique_ptr spm ( - make_production_system_package_manager (o, - host_triplet, - o.distribution (), - o.architecture ())); - if (spm == nullptr) - { - fail << "no standard distribution package manager for this host " - << "or it is not yet supported" << - info << "consider specifying alternative distribution package " - << "manager with --distribution" << - info << "specify --distribution=archive to generate installation " - << "archive" << - info << "consider specifying --os-release-* if unable to correctly " - << "auto-detect host operating system"; - } + rs.push_back (generate (pns, true /* first */)); - // Note that we pass type from here in case one day we want to provide an - // option to specify/override it (along with languages). Note that there - // will probably be no way to override type for dependencies. + // Generate dependencies, if requested. // - paths r (spm->generate (pkgs, deps, vars, db.config, pm, type, langs, rec)); + if (rec && rec == recursive_mode::separate) + generate_deps (rs.back ().deps, generate_deps); - if (r.empty ()) + t.commit (); + + if (rs.front ().bins.empty ()) return 0; // Assume prepare-only mode or similar. if (verb && !o.no_result ()) { - const selected_package& p (*pkgs.front ().selected); - - diag_record dr (text); - const string& d (o.distribution_specified () ? o.distribution () : spm->os_release.name_id); - dr << "generated " << d << " package for " << p.name << '/' << p.version - << ':'; + for (auto b (rs.begin ()), i (b); i != rs.end (); ++i) + { + const selected_package& p (*i->pkg); + + diag_record dr (text); - for (const path& p: r) - dr << "\n " << p; + dr << "generated " << d << " package for " + << (i != b ? "dependency " : "") + << p.name << '/' << p.version << ':'; + + for (const path& p: i->bins) + dr << "\n " << p; + } } return 0; diff --git a/bpkg/system-package-manager-archive.cxx b/bpkg/system-package-manager-archive.cxx index 4b4fd7f..384a0d5 100644 --- a/bpkg/system-package-manager-archive.cxx +++ b/bpkg/system-package-manager-archive.cxx @@ -43,7 +43,7 @@ namespace bpkg else target = host; - arch = target.cpu; // Set in case queried by someone else. + arch = target.string (); // Set in case queried by someone else. } // env --chdir= tar|zip ... . @@ -294,7 +294,8 @@ namespace bpkg const package_manifest& pm, const string& pt, const small_vector& langs, - optional recur) + optional recursive_full, + bool first) { tracer trace ("system_package_manager_archive::generate"); @@ -457,9 +458,7 @@ namespace bpkg // since we know dependencies cannot be spread over multiple linked // configurations. // - string scope (!recur || *recur == recursive_mode::full - ? "project" - : "weak"); + string scope (!recursive_full || *recursive_full ? "project" : "weak"); // The plan is to create the archive directory (with the same name as the // archive base; we call it "destination directory") inside the output @@ -471,16 +470,13 @@ namespace bpkg // Also, by default, we are going to keep all the intermediate files on // failure for troubleshooting. // - if (exists (out)) + if (first && exists (out) && !empty (out)) { - if (!empty (out)) - { - if (!ops->wipe_output ()) - fail << "output root directory " << out << " is not empty" << - info << "use --wipe-output to clean it up but be careful"; + if (!ops->wipe_output ()) + fail << "output root directory " << out << " is not empty" << + info << "use --wipe-output to clean it up but be careful"; - rm_r (out, false); - } + rm_r (out, false); } // NOTE: THE BELOW DESCRIPTION IS ALSO REWORDED IN BPKG-PKG-BINDIST(1). diff --git a/bpkg/system-package-manager-archive.hxx b/bpkg/system-package-manager-archive.hxx index 0d27d85..2792e7c 100644 --- a/bpkg/system-package-manager-archive.hxx +++ b/bpkg/system-package-manager-archive.hxx @@ -25,7 +25,8 @@ namespace bpkg const package_manifest&, const string&, const small_vector&, - optional) override; + optional, + bool) override; virtual optional pkg_status (const package_name&, const available_packages*) override; diff --git a/bpkg/system-package-manager-debian.cxx b/bpkg/system-package-manager-debian.cxx index f0a34a4..2f11701 100644 --- a/bpkg/system-package-manager-debian.cxx +++ b/bpkg/system-package-manager-debian.cxx @@ -1915,7 +1915,8 @@ namespace bpkg const package_manifest& pm, const string& pt, const small_vector& langs, - optional recur) + optional recursive_full, + bool first) { tracer trace ("system_package_manager_debian::generate"); @@ -2112,9 +2113,7 @@ namespace bpkg // since we know dependencies cannot be spread over multiple linked // configurations. // - string scope (!recur || *recur == recursive_mode::full - ? "project" - : "weak"); + string scope (!recursive_full || *recursive_full ? "project" : "weak"); // Get the map of files that will end up in the binary packages. // @@ -2148,16 +2147,13 @@ namespace bpkg // Also, by default, we are going to keep all the intermediate files on // failure for troubleshooting. // - if (exists (out)) + if (first && exists (out) && !empty (out)) { - if (!empty (out)) - { - if (!ops_->wipe_output ()) - fail << "output root directory " << out << " is not empty" << - info << "use --wipe-output to clean it up but be careful"; + if (!ops_->wipe_output ()) + fail << "output root directory " << out << " is not empty" << + info << "use --wipe-output to clean it up but be careful"; - rm_r (out, false); - } + rm_r (out, false); } // Normally the source directory is called - diff --git a/bpkg/system-package-manager-debian.hxx b/bpkg/system-package-manager-debian.hxx index cf7b1c3..877882e 100644 --- a/bpkg/system-package-manager-debian.hxx +++ b/bpkg/system-package-manager-debian.hxx @@ -141,7 +141,8 @@ namespace bpkg const package_manifest&, const string&, const small_vector&, - optional) override; + optional, + bool) override; public: // Expect os_release::name_id to be "debian" or os_release::like_ids to diff --git a/bpkg/system-package-manager-fedora.cxx b/bpkg/system-package-manager-fedora.cxx index 6ace1d2..7bcb51f 100644 --- a/bpkg/system-package-manager-fedora.cxx +++ b/bpkg/system-package-manager-fedora.cxx @@ -2281,7 +2281,8 @@ namespace bpkg const package_manifest& pm, const string& pt, const small_vector& langs, - optional recur) + optional recursive_full, + bool /* first */) { tracer trace ("system_package_manager_fedora::generate"); @@ -2739,9 +2740,7 @@ namespace bpkg // since we know dependencies cannot be spread over multiple linked // configurations. // - string scope (!recur || *recur == recursive_mode::full - ? "project" - : "weak"); + string scope (!recursive_full || *recursive_full ? "project" : "weak"); // Get the map of files that will end up in the binary packages. // diff --git a/bpkg/system-package-manager-fedora.hxx b/bpkg/system-package-manager-fedora.hxx index 53106d8..cedf295 100644 --- a/bpkg/system-package-manager-fedora.hxx +++ b/bpkg/system-package-manager-fedora.hxx @@ -207,7 +207,8 @@ namespace bpkg const package_manifest&, const string&, const small_vector&, - optional) override; + optional, + bool) override; public: // Expect os_release::name_id to be "fedora" or os_release::like_ids to diff --git a/bpkg/system-package-manager.hxx b/bpkg/system-package-manager.hxx index 6cdb2d4..52fb16b 100644 --- a/bpkg/system-package-manager.hxx +++ b/bpkg/system-package-manager.hxx @@ -158,7 +158,8 @@ namespace bpkg // Generate a binary distribution package. See the pkg-bindist(1) man page // for background and the pkg_bindist() function implementation for - // details. + // details. The recursive_full argument corresponds to the --recursive + // auto (present false) and full (present true) modes. // // The available packages are loaded for all the packages in pkgs and // deps. For non-system packages (so for all in pkgs) there is always a @@ -179,10 +180,14 @@ namespace bpkg // the package. // // Return the list of paths to binary packages and any other associated - // files (build metadata, etc) that could be useful for consumption of - // binary packages. If the result is empty, assume the prepare-only mode - // (or similar) with appropriate result diagnostics having been already - // issued. + // files (build metadata, etc) that could be useful for their consumption. + // If the result is empty, assume the prepare-only mode (or similar) with + // appropriate result diagnostics having been already issued. + // + // Note that this function may be called multiple times in the + // --recursive=separate mode. In this case the first argument indicates + // whether this is the first call (can be used, for example, to adjust the + // --wipe-output semantics). // struct package { @@ -193,8 +198,6 @@ namespace bpkg using packages = vector; - enum class recursive_mode {auto_, full}; - virtual paths generate (const packages& pkgs, const packages& deps, @@ -203,7 +206,8 @@ namespace bpkg const package_manifest&, const string& type, const small_vector&, - optional) = 0; + optional recursive_full, + bool first) = 0; public: bpkg::os_release os_release; -- cgit v1.1