From c84c3bda276e6b07ab784521de2634376286e76d Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Mon, 6 Mar 2023 08:48:43 +0200 Subject: Various cleanups and fixes --- bpkg/pkg-bindist.cli | 20 +++ bpkg/pkg-bindist.cxx | 33 +++-- bpkg/system-package-manager-debian.cxx | 219 ++++++++++++++++++++++++++++----- bpkg/system-package-manager-debian.hxx | 6 +- bpkg/system-package-manager-fedora.cxx | 4 +- bpkg/system-package-manager-fedora.hxx | 2 +- bpkg/system-package-manager.cxx | 3 +- bpkg/system-package-manager.hxx | 19 ++- bpkg/utility.hxx | 4 + bpkg/utility.txx | 97 ++++++++------- 10 files changed, 313 insertions(+), 94 deletions(-) diff --git a/bpkg/pkg-bindist.cli b/bpkg/pkg-bindist.cli index eef0c1e..527cc8b 100644 --- a/bpkg/pkg-bindist.cli +++ b/bpkg/pkg-bindist.cli @@ -55,6 +55,26 @@ namespace bpkg { "\h|PKG-BINDIST DEBIAN OPTIONS|" + bool --debian-prepare-only + { + "Prepare all the package metadata files (\cb{control}, \cb{rules}, etc) + but do not invoke \cb{bpkg-buildpackage} to generate the binary + package, printing its command line instead unless requested to be + quiet. Implies \cb{--keep-out}." + } + + string --debian-buildflags = "assign" + { + "", + "Package build flags (\cb{dpkg-buildflags}) usage mode. Valid + values are \cb{assign} (use the build flags instead of configured), + \cb{append} (use the build flags in addition to configured, putting + them last), \cb{prepend} (use the build flags in addition to + configured, putting them first), and \cb{ignore} (ignore build + flags). The default mode is \cb{assign}. Note that compiler mode + options, if any, are used as configured." + } + string --debian-section { "", diff --git a/bpkg/pkg-bindist.cxx b/bpkg/pkg-bindist.cxx index 6db7cdb..d68f60b 100644 --- a/bpkg/pkg-bindist.cxx +++ b/bpkg/pkg-bindist.cxx @@ -102,6 +102,7 @@ namespace bpkg // static void collect_dependencies (const common_options& co, + database& db, packages* pkgs, packages& deps, const string& type, @@ -116,12 +117,21 @@ namespace bpkg // We only consider dependencies from target configurations, similar // to pkg-install. // - database& db (ld.database ()); - if (db.type == host_config_type || db.type == build2_config_type) + database& pdb (ld.database ()); + if (pdb.type == host_config_type || pdb.type == build2_config_type) continue; shared_ptr d (ld.load ()); + // Packaging stuff that is spread over multiple configurations is just + // to hairy so we don't support it. Specifically, it becomes tricky to + // override build options since using a global override will also affect + // host/build2 configurations. + // + if (db != pdb) + fail << "dependency package " << *d << " belongs to different " + << "configuration " << pdb.config_orig; + // The selected package can only be configured if all its dependencies // are configured. // @@ -164,7 +174,7 @@ namespace bpkg } if (recursive && !sys) - collect_dependencies (co, pkgs, deps, type, langs, p, recursive); + collect_dependencies (co, db, pkgs, deps, type, langs, p, recursive); } } } @@ -327,6 +337,7 @@ namespace bpkg // for us to detect this and it's better to over- than under-specify. // collect_dependencies (o, + db, (rec ? *rec == recursive_mode::full ? &pkgs : nullptr : &deps), @@ -374,16 +385,22 @@ namespace bpkg // option to specify/override it (along with languages). Note that there // will probably be no way to override type for dependencies. // - spm->generate (pkgs, deps, vars, pm, type, langs, out, rec); + paths r (spm->generate (pkgs, deps, vars, pm, type, langs, out, rec)); + + if (r.empty ()) + return 0; // Assume prepare-only mode or similar. - // @@ TODO: change the output, maybe to something returned by spm? - // if (verb && !o.no_result ()) { const selected_package& p (*pkgs.front ().selected); - text << "generated " << spm->os_release.name_id << " package for " - << p.name << '/' << p.version; + diag_record dr (text); + + dr << "generated " << spm->os_release.name_id << " package for " + << p.name << '/' << p.version << ':'; + + for (const path& p: r) + dr << "\n " << p; } return 0; diff --git a/bpkg/system-package-manager-debian.cxx b/bpkg/system-package-manager-debian.cxx index 15825ef..a3f2721 100644 --- a/bpkg/system-package-manager-debian.cxx +++ b/bpkg/system-package-manager-debian.cxx @@ -1845,7 +1845,11 @@ namespace bpkg // Specifially, we can override the install target to invoke the build // system and install all the packages directly from their bpkg locations. // - void system_package_manager_debian:: + // Note that the -dbgsym packages are generated by default and all we need + // to do from our side is to compile with debug information (-g), failed + // which we get a warning from debhelper. + // + paths system_package_manager_debian:: generate (const packages& pkgs, const packages& deps, const strings& vars, @@ -1855,6 +1859,8 @@ namespace bpkg const dir_path& out, optional recur) { + tracer trace ("system_package_manager_debian::generate"); + assert (!langs.empty ()); // Should be effective. const shared_ptr& sp (pkgs.front ().selected); @@ -1935,6 +1941,7 @@ namespace bpkg sdeps.push_back (move (s)); } + if (verb >= 3) { auto print_status = [] (diag_record& dr, const package_status& s) { @@ -1946,16 +1953,24 @@ namespace bpkg << ' ' << s.system_version; }; - diag_record dr (text); - print_status (dr, st); + { + diag_record dr (trace); + dr << "package: "; + print_status (dr, st); + } for (const package_status& st: sdeps) { - dr << "\n "; + diag_record dr (trace); + dr << "dependency: "; print_status (dr, st); } } + if (!st.dbg.empty ()) + fail << "generation of obsolete manual -dbg packages not supported" << + info << "use automatic -dbgsym packages instead"; + // We override every config.install.* variable in order not to pick // anything configured. Note that we add some more in the rules file // below. @@ -2023,12 +2038,13 @@ namespace bpkg for (const string& v: vars) config.push_back (v); - // Note that we have to use global install scope for the auto recursive - // mode since things can be spread over multiple linked configurations. + // Note that we can use weak install scope for the auto recursive mode + // since we know dependencies cannot be spread over multiple linked + // configurations. // string scope (!recur || *recur == recursive_mode::full ? "project" - : "global"); + : "weak"); // Get the map of files that will end up in the binary packages. // @@ -2040,10 +2056,19 @@ namespace bpkg if (ies.empty ()) fail << "specified package(s) do not install any files"; -#if 0 - for (const auto& p: ies) - text << p.first; -#endif + if (verb >= 4) + { + for (const auto& p: ies) + { + diag_record dr (trace); + dr << "installed entry: " << p.first; + + if (p.second.target != nullptr) + dr << " -> " << p.second.target->first; // Symlink. + else + dr << " " << p.second.mode; + } + } // Start assembling the package "source" directory. // @@ -2080,7 +2105,7 @@ namespace bpkg // Note that we try to do a reasonably thorough job (e.g., filling in // sections, etc) with the view that this can be used as a starting point // for manual packaging (and perhaps we could add a mode for this in the - // future). + // future, call it "starting point" mode). // // Also note that this file supports variable substitutions (for example, // ${binary:Version}) as described in deb-substvars(5). While we could do @@ -2209,6 +2234,12 @@ namespace bpkg if (!depends.empty ()) depends += ", "; + // Note that the constraints will include build metadata (e.g., + // ~debian10). While it may be tempting to strip it, we cannot since + // the order is inverse. We could just make it empty `~`, though + // that will look a bit strange. But keeping it shouldn't cause any + // issues. + // depends += st.main + " (>= " + st.system_version + ')'; } @@ -2314,6 +2345,8 @@ namespace bpkg << " This package contains the documentation." << '\n'; } + // Keep this in case we want to support it in the "starting point" mode. + // if (!st.dbg.empty ()) { string depends (st.main + " (= ${binary:Version})"); @@ -2410,8 +2443,6 @@ namespace bpkg // // ,
:: + // - // @@ We may need to "complete" the maintainer if it's just an email. - // timestamp now (system_clock::now ()); os << " -- " << maintainer << " "; std::locale l (os.imbue (std::locale ("C"))); @@ -2441,7 +2472,7 @@ namespace bpkg // Note also that there is currently no way for us to get accurate // copyright information. // - // @@ Also, strictly speaking, in the recursive mode, we should collect + // @@ TODO: Strictly speaking, in the recursive mode, we should collect // licenses of all the dependencies we are bundling. // path copyr (deb / "copyright"); @@ -2542,6 +2573,10 @@ namespace bpkg path rules (deb / "rules"); try { + bool lang_c (lang ("c")); + bool lang_cxx (lang ("c++")); + bool lang_cc (lang ("cc")); + // See fdopen() for details (umask, etc). // permissions ps (permissions::ru | permissions::wu | permissions::xu | @@ -2557,6 +2592,11 @@ namespace bpkg // See debhelper(7) for details on these. // + // Note that there is also the DEB_BUILD_OPTIONS=terse option. Perhaps + // for the "starting point" mode we should base DH_* values as well as + // the build system verbosity below on that value. See debian/rules in + // upstream mariadb for what looks like a sensible setup. + // if (verb == 0) os << "export DH_QUIET := 1\n" << '\n'; @@ -2568,15 +2608,32 @@ namespace bpkg os << "export DH_VERBOSE := 1\n" << '\n'; - // We could have instead included architecture.mk but let's avoid an - // extra dependency (most packages that we sampled do it directly). + // We could have instead called dpkg-architecture directly but seeing + // that we are also include buildflags.mk below, might as well use + // architecture.mk (in the packages that we sampled you see both + // approaches). // - // @@ Add --debian-no-multiarch? Will get quite messy (see .install - // files). - // - os << "export DEB_HOST_MULTIARCH ?= $(shell dpkg-architecture -qDEB_HOST_MULTIARCH)\n" + os << "# DEB_HOST_* (DEB_HOST_MULTIARCH, etc)" + << "#" << '\n' + << "include /usr/share/dpkg/architecture.mk" << '\n' << '\n'; + if (ops_->debian_buildflags () != "ignore") + { + // While we could have called dpkg-buildflags directly, including + // buildflags.mk instead appears to be the standard practice. + // + // Note that theses flags are not limited to C-based languages (for + // example, they also cover Assembler, Fortran, and potentially others + // in the future). + // + os << "# *FLAGS (CFLAGS, CXXFLAGS, etc)" + << "#" << '\n' + << "export DEB_BUILD_MAINT_OPTIONS := hardening=+all" << '\n' + << "include /usr/share/dpkg/buildflags.mk" << '\n' + << '\n'; + } + // The debian/tmp/ subdirectory appears to be the canonical destination // directory (see dh_auto_install(1) for details). // @@ -2589,14 +2646,14 @@ namespace bpkg // See --jobs documentation in dpkg-buildpackage(1) for details on // parallel=N. // - // @@ Need to map verbosity! Also looks like progress is disabled - // (probably because stderr redirected to pipe). @@ No, there is - // progress. Maybe just keep, doesn't seem harmful. Or pass ours. - // // Note: should be consistent with the invocation in installed_entries() // above. // + cstrings verb_args; string verb_arg; + map_verb_b (*ops_, verb_b::normal, verb_args, verb_arg); + os << "b := " << search_b (*ops_).effect_string (); + for (const char* o: verb_args) os << ' ' << o; for (const string& o: ops_->build_option ()) os << ' ' << o; os << '\n' << '\n' @@ -2622,10 +2679,63 @@ namespace bpkg // If this is a C-based language, add rpath for private installation. // - if (priv && (lang ("c") || lang ("c++") || lang ("cc"))) + if (priv && (lang_c || lang_cxx || lang_cc)) os << "config += config.bin.rpath='/usr/lib/$(DEB_HOST_MULTIARCH)/" << pn << "/'" << '\n'; + // Add build flags. + // + if (ops_->debian_buildflags () != "ignore") + { + const string& m (ops_->debian_buildflags ()); + + string o (m == "assign" ? "=" : + m == "append" ? "+=" : + m == "prepend" ? "=+" : ""); + + if (o.empty ()) + fail << "unknown --debian-buildflags option value '" << m << "'"; + + // Note that config.cc.* doesn't play well with the append/prepend + // modes because the orders are: + // + // x.poptions cc.poptions + // cc.coptions x.coptions + // cc.loptions x.loptions + // + // Oh, well, hopefully it will be close enough for most cases. + // + // Note also that there are compiler mode options that are not + // overridden. + // + if (o == "=" && (lang_c || lang_cxx || lang_cc)) + { + os << "config += config.cc.poptions='[null]'" << '\n' + << "config += config.cc.coptions='[null]'" << '\n' + << "config += config.cc.loptions='[null]'" << '\n'; + } + + if (lang_c || lang_cc) + { + // @@ TODO: OBJCFLAGS (we currently don't have separate options). + + os << "config += config.c.poptions" << o << "'$(CPPFLAGS)'" << '\n' + << "config += config.c.coptions" << o << "'$(CFLAGS)'" << '\n' + << "config += config.c.loptions" << o << "'$(LDFLAGS)'" << '\n'; + } + + if (lang_cxx || lang_cc) + { + // @@ TODO: OBJCXXFLAGS (we currently don't have separate options). + + os << "config += config.cxx.poptions" << o << "'$(CPPFLAGS)'" << '\n' + << "config += config.cxx.coptions" << o << "'$(CXXFLAGS)'" << '\n' + << "config += config.cxx.loptions" << o << "'$(LDFLAGS)'" << '\n'; + } + + // @@ TODO: ASFLAGS (when we have assembler support). + } + // Keep last to allow user-specified configuration variables to override // anything. // @@ -2987,11 +3097,9 @@ namespace bpkg // Note that there doesn't seem to be any way to control its verbosity or // progress. // - // @@ Buildinfo stuff is fuzzy. There is also debug symbols warnings. - // - // @@ Why does stuff keep recompiling on every run (e.g., byacc)? Running - // the same commands from the command line does not trigger rebuild. - // Could dpkg-buildpackage somehow forcing rebuild? Via environment? + // Note also that dpkg-buildpackage causes recompilation on every run by + // changing the SOURCE_DATE_EPOCH environment variable (which we track for + // changes since it affects GCC). // cstrings args { "dpkg-buildpackage", @@ -3011,6 +3119,21 @@ namespace bpkg args.push_back (nullptr); + if (ops_->debian_prepare_only ()) + { + if (verb >= 1) + { + diag_record dr (text); + + dr << "prepared " << src << + text << "command line: "; + + print_process (dr, args); + } + + return paths {}; + } + try { process_path pp (process::path_search (args[0])); @@ -3060,5 +3183,39 @@ namespace bpkg { rm_r (src); } + + // Collect and return the binary package paths. + // + paths r; + auto add = [&out, &r] (const string& n, bool opt = false) + { + path p (out / n); + + if (exists (p)) + r.push_back (move (p)); + else if (!opt) + fail << "expected output file " << p << " does not exist"; + }; + + // The resulting .deb file names have the __.deb + // form. If the package is architecture-independent, then is the + // special `all` value. + // + const string& ver (st.system_version); + + add (st.main + '_' + ver + '_' + arch + ".deb"); + add (st.main + "-dbgsym_" + ver + '_' + arch + ".deb", true); + if (!st.dev.empty ()) add (st.dev + '_' + ver + '_' + arch + ".deb"); + if (!st.doc.empty ()) add (st.doc + '_' + ver + "_all.deb"); + if (!st.common.empty ()) add (st.common + '_' + ver + "_all.deb"); + + // Besides the binary packages (.deb) we also get the .buildinfo and + // .changes files, which could be useful. Note that their names are based + // on the source package name. + // + add (pn.string () + '_' + ver + '_' + arch + ".buildinfo"); + add (pn.string () + '_' + ver + '_' + arch + ".changes"); + + return r; } } diff --git a/bpkg/system-package-manager-debian.hxx b/bpkg/system-package-manager-debian.hxx index 8398b70..b82cfd2 100644 --- a/bpkg/system-package-manager-debian.hxx +++ b/bpkg/system-package-manager-debian.hxx @@ -39,6 +39,10 @@ namespace bpkg // Note that while most library package names in Debian start with lib (per // the policy), there are exceptions (e.g., zlib1g zlib1g-dev). // + // Also note that manual -dbg packages are obsolete in favor of automatic + // -dbgsym packages from Debian 9. So while we support -dbg for consumption, + // we only generate -dbgsym. + // // Based on that, it seems our best bet when trying to automatically map our // library package name to Debian package names is to go for the -dev // package first and figure out the shared library package from that based @@ -129,7 +133,7 @@ namespace bpkg virtual void pkg_install (const vector&) override; - virtual void + virtual paths generate (const packages&, const packages&, const strings&, diff --git a/bpkg/system-package-manager-fedora.cxx b/bpkg/system-package-manager-fedora.cxx index db56cd4..45b4fdd 100644 --- a/bpkg/system-package-manager-fedora.cxx +++ b/bpkg/system-package-manager-fedora.cxx @@ -1794,7 +1794,7 @@ namespace bpkg } } - void system_package_manager_fedora:: + paths system_package_manager_fedora:: generate (const packages&, const packages&, const strings&, @@ -1804,5 +1804,7 @@ namespace bpkg const dir_path&, optional) { + paths r; + return r; } } diff --git a/bpkg/system-package-manager-fedora.hxx b/bpkg/system-package-manager-fedora.hxx index 6d919c7..792c3f3 100644 --- a/bpkg/system-package-manager-fedora.hxx +++ b/bpkg/system-package-manager-fedora.hxx @@ -196,7 +196,7 @@ namespace bpkg virtual void pkg_install (const vector&) override; - virtual void + virtual paths generate (const packages&, const packages&, const strings&, diff --git a/bpkg/system-package-manager.cxx b/bpkg/system-package-manager.cxx index eb48727..4dc9bf7 100644 --- a/bpkg/system-package-manager.cxx +++ b/bpkg/system-package-manager.cxx @@ -775,7 +775,8 @@ namespace bpkg if (!(e = p.next ()) || *e != event::string) fail << "path member string value expected"; - path ep (p.value ()); // Assuming normalized. + path ep (p.value ()); + assert (ep.absolute () && ep.normalized (false /* separators */)); if (t == "file" || t == "directory") { diff --git a/bpkg/system-package-manager.hxx b/bpkg/system-package-manager.hxx index 34af752..65f4b2d 100644 --- a/bpkg/system-package-manager.hxx +++ b/bpkg/system-package-manager.hxx @@ -154,12 +154,17 @@ namespace bpkg virtual void pkg_install (const vector&) = 0; - // Generate a binary distribution package. @@ TODO: doc more + // Generate a binary distribution package. See the pkg-bindist(1) man page + // for background and the pkg_bindist() function implementation for + // details. // // 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 // single available package that corresponds to the selected package. The - // out_root is only set for packages in pkgs. + // out_root is only set for packages in pkgs. Note also that all the + // packages in pkgs and deps are guaranteed to belong to the same build + // configuration (as opposed to being spread over multiple linked + // configurations). // // The passed package manifest corresponds to the first package in pkgs // (normally used as a source of additional package metadata such as @@ -171,10 +176,12 @@ namespace bpkg // list contains every language that is used by anything that ends up in // the package. // - // See the pkg-bindist(1) man page and the pkg_bindist() function - // implementation for background and details. + // Return the list of paths to binary packages and any other associated + // files (build metadata, etc). If the result is empty, assume the + // prepare-only mode (or similar) with appropriate result diagnostics + // having been already issued. // - struct package // @@ TODO: any better name? + struct package { shared_ptr selected; available_packages available; @@ -185,7 +192,7 @@ namespace bpkg enum class recursive_mode {auto_, full}; - virtual void + virtual paths generate (const packages& pkgs, const packages& deps, const strings& vars, diff --git a/bpkg/utility.hxx b/bpkg/utility.hxx index 79534f0..bb264ba 100644 --- a/bpkg/utility.hxx +++ b/bpkg/utility.hxx @@ -244,6 +244,10 @@ namespace bpkg normal // Run normally (at verbosity 1). }; + template + void + map_verb_b (const common_options&, verb_b, V& args, string& verb_arg); + const char* name_b (const common_options&); diff --git a/bpkg/utility.txx b/bpkg/utility.txx index 3fe1d72..a21325c 100644 --- a/bpkg/utility.txx +++ b/bpkg/utility.txx @@ -7,6 +7,56 @@ namespace bpkg { // *_b() // + template + void + map_verb_b (const common_options& co, verb_b v, V& ops, string& verb_arg) + { + // Map verbosity level. If we are running quiet or at level 1, + // then run build2 quiet. Otherwise, run it at the same level + // as us. + // + bool progress (co.progress ()); + bool no_progress (co.no_progress ()); + + if (verb == 0) + { + ops.push_back ("-q"); + no_progress = false; // Already suppressed with -q. + } + else if (verb == 1) + { + if (v != verb_b::normal) + { + ops.push_back ("-q"); + + if (!no_progress) + { + if (v == verb_b::progress && stderr_term) + { + ops.push_back ("--progress"); + progress = false; // The option is already added. + } + } + else + no_progress = false; // Already suppressed with -q. + } + } + else if (verb == 2) + ops.push_back ("-v"); + else + { + verb_arg = to_string (verb); + ops.push_back ("--verbose"); + ops.push_back (verb_arg.c_str ()); + } + + if (progress) + ops.push_back ("--progress"); + + if (no_progress) + ops.push_back ("--no-progress"); + } + template process start_b (const common_options& co, @@ -24,51 +74,8 @@ namespace bpkg // NOTE: see custom versions in system_package_manager* if adding // anything new here (search for search_b()). - // Map verbosity level. If we are running quiet or at level 1, - // then run build2 quiet. Otherwise, run it at the same level - // as us. - // - string vl; - bool progress (co.progress ()); - bool no_progress (co.no_progress ()); - - if (verb == 0) - { - ops.push_back ("-q"); - no_progress = false; // Already suppressed with -q. - } - else if (verb == 1) - { - if (v != verb_b::normal) - { - ops.push_back ("-q"); - - if (!no_progress) - { - if (v == verb_b::progress && stderr_term) - { - ops.push_back ("--progress"); - progress = false; // The option is already added. - } - } - else - no_progress = false; // Already suppressed with -q. - } - } - else if (verb == 2) - ops.push_back ("-v"); - else - { - vl = to_string (verb); - ops.push_back ("--verbose"); - ops.push_back (vl.c_str ()); - } - - if (progress) - ops.push_back ("--progress"); - - if (no_progress) - ops.push_back ("--no-progress"); + string verb_arg; + map_verb_b (co, v, ops, verb_arg); // Forward our --[no]diag-color options. // -- cgit v1.1