diff options
| -rw-r--r-- | bpkg/package.cxx | 71 | ||||
| -rw-r--r-- | bpkg/package.hxx | 38 | ||||
| -rw-r--r-- | bpkg/pkg-build-collect.cxx | 1 | ||||
| -rw-r--r-- | bpkg/pkg-build.cxx | 2 | ||||
| -rw-r--r-- | bpkg/pkg-checkout.cxx | 29 | ||||
| -rw-r--r-- | bpkg/pkg-purge.cxx | 1 | ||||
| -rw-r--r-- | bpkg/pkg-unpack.cxx | 60 | ||||
| -rw-r--r-- | bpkg/pkg-unpack.hxx | 6 | ||||
| -rw-r--r-- | tests/pkg-build.testscript | 58 | ||||
| -rw-r--r-- | tests/rep-fetch.testscript | 11 |
10 files changed, 115 insertions, 162 deletions
diff --git a/bpkg/package.cxx b/bpkg/package.cxx index fe04248..c98e7a4 100644 --- a/bpkg/package.cxx +++ b/bpkg/package.cxx @@ -389,45 +389,54 @@ namespace bpkg false /* iteration */)) return nullopt; - string mc (package_checksum (o, d, pi)); + bool changed (!p->external ()); - // The selected package must not be "simulated" (see pkg-build for - // details). + // If the selected package is not external, then increment the iteration + // number to make the external package preferable. Note that for such + // packages the manifest/subprojects and buildfiles checksums are absent. // - assert (p->manifest_checksum); - - bool changed (mc != *p->manifest_checksum); - - // If the manifest hasn't changed and the package has buildfile clauses in - // the dependencies, then check if the buildfiles haven't changed either. - // - if (!changed && p->buildfiles_checksum) + if (!changed) { - // Always calculate the checksum over the buildfiles since the package - // is external. + // The selected package must not be "simulated" (see pkg-build for + // details). // - changed = package_buildfiles_checksum ( - nullopt /* bootstrap_build */, - nullopt /* root_build */, - {} /* buildfiles */, - d) != *p->buildfiles_checksum; - } + assert (p->manifest_checksum); - // If the manifest hasn't changed but the selected package points to an - // external source directory, then we also check if the directory have - // moved. - // - if (!changed && p->external ()) - { - dir_path src_root (p->effective_src_root (db.config)); + changed = (package_checksum (o, d, pi) != *p->manifest_checksum); - // We need to complete and normalize the source directory as it may - // generally be completed against the configuration directory (unlikely - // but possible), that can be relative and/or not normalized. + // If the manifest hasn't changed and the package has buildfile clauses + // in the dependencies, then check if the buildfiles haven't changed + // either. // - normalize (src_root, "package source"); + if (!changed && p->buildfiles_checksum) + { + // Always calculate the checksum over the buildfiles since the package + // is external. + // + changed = package_buildfiles_checksum ( + nullopt /* bootstrap_build */, + nullopt /* root_build */, + {} /* buildfiles */, + d) != *p->buildfiles_checksum; + } + + // If the manifest hasn't changed but the selected package points to an + // external source directory, then we also check if the directory have + // moved. + // + if (!changed) + { + dir_path src_root (p->effective_src_root (db.config)); - changed = src_root != normalize (d, "package source"); + // We need to complete and normalize the source directory as it may + // generally be completed against the configuration directory + // (unlikely but possible), that can be relative and/or not + // normalized. + // + normalize (src_root, "package source"); + + changed = src_root != normalize (d, "package source"); + } } return !changed diff --git a/bpkg/package.hxx b/bpkg/package.hxx index 3283be5..060f13a 100644 --- a/bpkg/package.hxx +++ b/bpkg/package.hxx @@ -1237,9 +1237,9 @@ namespace bpkg // package version revision increment. In particular, new subprojects // should trigger the package reconfiguration. // - // Must be present if the source directory is present, unless the object - // is created/updated during the package build simulation (see pkg-build - // for details). Note that during the simulation the manifest may not be + // Only present for external packages, unless the objects are + // created/updated during the package build simulation (see pkg-build for + // details). Note that during the simulation the manifest may not be // available. // // @@ Currently we don't consider subprojects recursively (would most @@ -1251,13 +1251,13 @@ namespace bpkg // optional<std::string> manifest_checksum; - // Absent if the package has no buildfile clauses in the dependencies. - // Otherwise, the checksum of the buildfiles calculated over the *-build - // manifest values or, if unspecified, the files in the package source - // directory. + // Only present for external packages which have buildfile clauses in the + // dependencies, unless the objects are created/updated during the package + // build simulation (see pkg-build for details). // - // Note that for external packages the checksum is always calculated over - // the files. This is "parallel" to the package skeleton logic. + // Note that the checksum is always calculated over the files rather than + // the *-build manifest values. This is "parallel" to the package skeleton + // logic. // optional<std::string> buildfiles_checksum; @@ -1429,15 +1429,17 @@ namespace bpkg // // - The package directory is considered an iteration of the package if this // upstream version and revision is already present (selected) in the - // configuration and has a source directory. If that's the case, then the - // specified directory path and the package checksum (see - // package_checksum() for details) are compared to the ones of the package - // present in the configuration. If both match, then the present package - // version (including its iteration, if any) is returned. Otherwise (the - // package has moved and/or the package information has changed), the - // present package version with the incremented iteration number is - // returned. Note that the directory path is matched only for the external - // selected packages. + // configuration and has a source directory. If that's the case and if the + // present version is not external (the package is being switched to a + // local potentially amended version), then the present package version + // with the incremented iteration number is returned. Otherwise (the + // present package is external), the specified directory path and the + // package checksum (see package_checksum() for details) are compared to + // the ones of the package present in the configuration. If both match, + // then the present package version (including its iteration, if any) is + // returned. Otherwise (the package has moved and/or the package + // information has changed), the present package version with the + // incremented iteration number is returned. // // - Only a single package iteration is valid per version in the // configuration. This, in particular, means that a package of the diff --git a/bpkg/pkg-build-collect.cxx b/bpkg/pkg-build-collect.cxx index 95e4eee..69c0eea 100644 --- a/bpkg/pkg-build-collect.cxx +++ b/bpkg/pkg-build-collect.cxx @@ -1411,7 +1411,6 @@ namespace bpkg (pkg.selected->version != pkg.available_version () || pkg.system)) { - for (const postponed_configuration& cfg: postponed_cfgs) { auto i (cfg.dependents.find (pk)); diff --git a/bpkg/pkg-build.cxx b/bpkg/pkg-build.cxx index 55fae96..2796550 100644 --- a/bpkg/pkg-build.cxx +++ b/bpkg/pkg-build.cxx @@ -5370,7 +5370,7 @@ namespace bpkg // Commits the transaction. // - sp = pkg_unpack (o, pdb, af.database (), t, ap->id.name, simulate); + sp = pkg_unpack (o, pdb, t, ap->id.name, simulate); if (result) text << "unpacked " << *sp << pdb; diff --git a/bpkg/pkg-checkout.cxx b/bpkg/pkg-checkout.cxx index 87579cc..9793c41 100644 --- a/bpkg/pkg-checkout.cxx +++ b/bpkg/pkg-checkout.cxx @@ -170,9 +170,6 @@ namespace bpkg const repository_location& rl (pl->repository_fragment->location); auto_rmdir rmd; - optional<string> mc; - optional<string> bc; - const dir_path& ord (output_root ? *output_root : c); dir_path d (ord / dir_path (n.string () + '-' + v.string ())); @@ -312,21 +309,6 @@ namespace bpkg "!config.dist.bootstrap=true", "config.dist.root='" + ord.representation () + '\'', bspec); - - mc = package_checksum (o, d, nullptr /* package_info */); - - // Calculate the buildfiles checksum if the package has any buildfile - // clauses in the dependencies. - // - // Note that the available package already has all the buildfiles - // loaded. - // - if ((p != nullptr && p->manifest_checksum == mc) - ? p->buildfiles_checksum.has_value () - : has_buildfile_clause (ap->dependencies)) - bc = package_buildfiles_checksum (ap->bootstrap_build, - ap->root_build, - ap->buildfiles); } if (p != nullptr) @@ -360,13 +342,16 @@ namespace bpkg if (p != nullptr) { + // Note: we can be replacing an external package and thus we reset the + // manifest/subprojects and buildfiles checksums. + // p->version = move (v); p->state = package_state::unpacked; p->repository_fragment = rl; p->src_root = move (d); p->purge_src = purge; - p->manifest_checksum = move (mc); - p->buildfiles_checksum = move (bc); + p->manifest_checksum = nullopt; + p->buildfiles_checksum = nullopt; pdb.update (p); } @@ -386,8 +371,8 @@ namespace bpkg false, move (d), // Source root. purge, // Purge directory. - move (mc), - move (bc), + nullopt, // No manifest/subprojects checksum. + nullopt, // No buildfiles checksum. nullopt, // No output directory yet. {}}); // No prerequisites captured yet. diff --git a/bpkg/pkg-purge.cxx b/bpkg/pkg-purge.cxx index 4fe040e..07951a6 100644 --- a/bpkg/pkg-purge.cxx +++ b/bpkg/pkg-purge.cxx @@ -50,6 +50,7 @@ namespace bpkg // p->src_root = nullopt; p->manifest_checksum = nullopt; + p->buildfiles_checksum = nullopt; if (archive) { diff --git a/bpkg/pkg-unpack.cxx b/bpkg/pkg-unpack.cxx index 8f01a8a..90ed331 100644 --- a/bpkg/pkg-unpack.cxx +++ b/bpkg/pkg-unpack.cxx @@ -162,7 +162,10 @@ namespace bpkg optional<string> mc; optional<string> bc; - if (!simulate) + // Only calculate the manifest/subprojects and buildfiles checksums for + // external packages (see selected_package::external() for details). + // + if (!simulate && (rl.empty () || rl.directory_based ())) { mc = package_checksum (o, d, pi); @@ -334,7 +337,6 @@ namespace bpkg shared_ptr<selected_package> pkg_unpack (const common_options& co, database& db, - database& rdb, transaction& t, const package_name& name, bool simulate) @@ -371,8 +373,6 @@ namespace bpkg fail << "package directory " << d << " already exists"; auto_rmdir arm; - optional<string> mc; - optional<string> bc; if (!simulate) { @@ -402,62 +402,11 @@ namespace bpkg { fail << "unable to extract " << a << " to " << c << ": " << e; } - - mc = package_checksum (co, d, nullptr /* package_info */); - - // Calculate the buildfiles checksum if the package has any buildfile - // clauses in the dependencies. - // - // Note that we may not have the available package (e.g., fetched as an - // existing package archive rather than from an archive repository), in - // which case we need to parse the manifest to retrieve the - // dependencies. This is unfortunate, but is probably not a big deal - // performance-wise given that this is not too common and we are running - // an archive unpacking process anyway. - // - shared_ptr<available_package> ap ( - rdb.find<available_package> (available_package_id (n, v))); - - if (ap != nullptr) - { - // Note that the available package already has all the buildfiles - // loaded. - // - if (has_buildfile_clause (ap->dependencies)) - bc = package_buildfiles_checksum (ap->bootstrap_build, - ap->root_build, - ap->buildfiles); - } - else - { - // Note that we don't need to translate the package version here since - // the manifest comes from an archive and so has a proper version - // already. - // - package_manifest m ( - pkg_verify (co, - d, - true /* ignore_unknown */, - false /* ignore_toolchain */, - false /* load_buildfiles */, - function<package_manifest::translate_function> ())); - - if (has_buildfile_clause (m.dependencies)) - bc = package_buildfiles_checksum (m.bootstrap_build, - m.root_build, - m.buildfiles, - d, - m.buildfile_paths, - m.alt_naming); - } } p->src_root = d.leaf (); // For now assuming to be in configuration. p->purge_src = true; - p->manifest_checksum = move (mc); - p->buildfiles_checksum = move (bc); - p->state = package_state::unpacked; db.update (p); @@ -522,7 +471,6 @@ namespace bpkg p = v.empty () ? pkg_unpack (o, db /* pdb */, - db /* rdb */, t, n, false /* simulate */) diff --git a/bpkg/pkg-unpack.hxx b/bpkg/pkg-unpack.hxx index 99d74e0..7394732 100644 --- a/bpkg/pkg-unpack.hxx +++ b/bpkg/pkg-unpack.hxx @@ -32,13 +32,9 @@ namespace bpkg // Unpack the fetched package and commit the transaction. // - // Note that both package and repository information configurations need to - // be passed. - // shared_ptr<selected_package> pkg_unpack (const common_options&, - database& pdb, - database& rdb, + database&, transaction&, const package_name&, bool simulate); diff --git a/tests/pkg-build.testscript b/tests/pkg-build.testscript index fc89df5..d30186a 100644 --- a/tests/pkg-build.testscript +++ b/tests/pkg-build.testscript @@ -4625,33 +4625,36 @@ test.arguments += --sys-no-query $backend_configured EOO - # No upgrade after turning a package from the archive-based repo - # into an external package. + # Iteration increment and upgrade after turning a package from the + # archive-based repo into an external package. # $* fax/ 2>>~%EOE%; - %info: .+fax-1.0.0.+ is up to date% - updated fax/1.0.0 + disfigured fax/1.0.0 + using fax/1.0.0#1 (external) + configured fax/1.0.0#1 + %info: .+dir.fax.+ is up to date% + updated fax/1.0.0#1 EOE $pkg_status -r >>"EOO"; - !fax configured !1.0.0 + !fax configured !1.0.0#1 $backend_configured EOO - # Upgrade after the package' buildfile is edited. + # Further upgrade after the package' buildfile is edited. # echo '' >+fax/build/root.build; $* fax/ 2>>~%EOE%; - disfigured fax/1.0.0 - using fax/1.0.0#1 (external) - configured fax/1.0.0#1 + disfigured fax/1.0.0#1 + using fax/1.0.0#2 (external) + configured fax/1.0.0#2 %info: .+fax.+ is up to date% - updated fax/1.0.0#1 + updated fax/1.0.0#2 EOE $pkg_status -r >>"EOO"; - !fax configured !1.0.0#1 + !fax configured !1.0.0#2 $backend_configured EOO @@ -4659,11 +4662,11 @@ test.arguments += --sys-no-query # $* fax/ 2>>~%EOE%; %info: .+fax.+ is up to date% - updated fax/1.0.0#1 + updated fax/1.0.0#2 EOE $pkg_status -r >>"EOO"; - !fax configured !1.0.0#1 + !fax configured !1.0.0#2 $backend_configured EOO @@ -4692,37 +4695,40 @@ test.arguments += --sys-no-query $backend_configured EOO - # No upgrade after turning a package from the archive-based repo - # into an external package. + # Iteration increment and upgrade after turning a package from the + # archive-based repo into an external package. # $rep_add --type dir fax/ && $rep_fetch; $* fax 2>>~%EOE%; - %info: .+fax-1.0.0.+ is up to date% - updated fax/1.0.0 + disfigured fax/1.0.0 + using fax/1.0.0#1 (external) + configured fax/1.0.0#1 + %info: .+dir.fax.+ is up to date% + updated fax/1.0.0#1 EOE $pkg_status -r >>"EOO"; - !fax configured 1.0.0 + !fax configured 1.0.0#1 $backend_configured EOO - # Upgrade after the package' buildfile is edited. + # Further upgrade after the package' buildfile is edited. # echo '' >+fax/build/root.build; $rep_fetch; $* fax 2>>~%EOE%; - disfigured fax/1.0.0 - using fax/1.0.0#1 (external) - configured fax/1.0.0#1 + disfigured fax/1.0.0#1 + using fax/1.0.0#2 (external) + configured fax/1.0.0#2 %info: .+fax.+ is up to date% - updated fax/1.0.0#1 + updated fax/1.0.0#2 EOE $pkg_status -r >>"EOO"; - !fax configured 1.0.0#1 + !fax configured 1.0.0#2 $backend_configured EOO @@ -4732,11 +4738,11 @@ test.arguments += --sys-no-query $* fax 2>>~%EOE%; %info: .+fax.+ is up to date% - updated fax/1.0.0#1 + updated fax/1.0.0#2 EOE $pkg_status -r >>"EOO"; - !fax configured 1.0.0#1 + !fax configured 1.0.0#2 $backend_configured EOO diff --git a/tests/rep-fetch.testscript b/tests/rep-fetch.testscript index b9b9e05..b713c0c 100644 --- a/tests/rep-fetch.testscript +++ b/tests/rep-fetch.testscript @@ -631,11 +631,15 @@ if! $remote : unchanged-external : + : Test that iteration is still incremented when a non-external package + : from a pkg repository is switched to the same unedited external + : package. + : { $clone_cfg && $rep_add $src/libhello-1.0.0; $* 2>!; - $pkg_status libhello >'libhello unpacked 1.0.0' + $pkg_status libhello >'libhello unpacked 1.0.0 available 1.0.0#1' } : changed-external @@ -655,6 +659,9 @@ if! $remote : git-rep : + : Test that iteration is still incremented when a non-external package + : from a git repository is switched to the same unedited external package. + : if ($git_supported && !$remote) { rep = $canonicalize([dir_path] $out_git/state0); @@ -668,7 +675,7 @@ if! $remote $rep_add $rep/style.git; $* 2>!; - $pkg_status style >"style unpacked 1.0.0"; + $pkg_status style >"style unpacked 1.0.0 available 1.0.0#1"; $pkg_purge style 2>"purged style/1.0.0" } |