Drop old implementation of package_skeleton::evaluate_reflect()

1 files changed, 0 insertions, 351 deletions

diff --git a/bpkg/package-skeleton.cxx b/bpkg/package-skeleton.cxx
index 8b4b09f..d58c846 100644
--- a/bpkg/package-skeleton.cxx
+++ b/bpkg/package-skeleton.cxx
@@ -865,8 +865,6 @@ namespace bpkg
     }
   }
 
-#if 1
-
   void package_skeleton::
   evaluate_reflect (const string& refl, pair<size_t, size_t> indexes)
   {
@@ -1179,355 +1177,6 @@ namespace bpkg
     }
   }
 
-#else
-
-  // @@ TMP: keep this in dep-config-neg but drop in master.
-
-  void package_skeleton::
-  evaluate_reflect (const string& refl, pair<size_t, size_t> indexes)
-  {
-    size_t depends_index (indexes.first);
-
-    // The reflect configuration variables are essentially overrides that will
-    // be passed on the command line when we configure the package. They could
-    // clash with configuration variables specified by the user (config_vars_)
-    // and it feels like user values should take precedence. Though one could
-    // also argue we should diagnose this case and fail not to cause more
-    // confusion.
-    //
-    // They could also clash with dependent configuration. Probably should be
-    // handled in the same way (it's just another type of "user"). Yes, since
-    // dependent_vars_ are entered as cmd line overrides, this is how they are
-    // treated.
-    //
-    // It seems like the most straightforward way to achieve the desired
-    // semantics with the mechanisms that we have (in other words, without
-    // inventing another "level" of overrides) is to evaluate the reflect
-    // fragment after loading root.build. This way it will (1) be able to use
-    // variables set by root.build in conditions, (2) override default values
-    // of configuration variables (and those loaded from config.build), and
-    // (3) be overriden by configuration variables specified by the user.
-    // Naturally, this approach is probably not without a few corner cases.
-    //
-    // We may also have configuration values from the previous reflect clause
-    // which we want to "factor in" before evaluating the next clause (enable,
-    // reflect etc.; so that they can use the previously reflected values or
-    // values that are derived from them in root.build). It seems like we have
-    // two options here: either enter them as true overrides similar to
-    // config_vars_ or just evaluate them similar to loading config.build
-    // (which, BTW, we might have, in case of an external package). The big
-    // problem with the former approach is that it will then prevent any
-    // further reflect clauses from modifying the same values.
-    //
-    // So overall it feels like we have iterative/compartmentalized
-    // configuration process. A feedback loop, in a sense. And it's the
-    // responsibility of the package author (who is in control of both
-    // root.build and manifest) to arrange for suitable compartmentalization.
-    //
-    try
-    {
-      // Note: similar in many respects to evaluate_enable().
-      //
-      using namespace build2;
-      using build2::fail;
-      using build2::info;
-      using build2::endf;
-
-      // Drop the state from the previous evaluation of prefer/accept if it's
-      // from the wrong position.
-      //
-      optional<size_t> dependency_var_prefixes_pending;
-      if (prefer_accept_)
-      {
-        if (*prefer_accept_ != indexes)
-        {
-          ctx_ = nullptr;
-          prefer_accept_ = nullopt;
-        }
-        else
-        {
-          // This could theoretically happen if we make a copy of the skeleton
-          // after evaluate_prefer_accept() and then attempt to continue with
-          // the call on the copy to evaluate_reflect() passing the same
-          // position. But it doesn't appear our usage should trigger this.
-          //
-          assert (ctx_ != nullptr);
-
-          dependency_var_prefixes_pending = dependency_var_prefixes_pending_;
-        }
-      }
-
-      scope& rs (load ());
-
-      istringstream is (refl);
-      is.exceptions (istringstream::failbit | istringstream::badbit);
-
-      path_name in ("<depends-reflect-clause>");
-      uint64_t il (1);
-
-      auto df = build2::make_diag_frame (
-        [this, &refl, &rs, depends_index] (const build2::diag_record& dr)
-        {
-          // Probably safe to assume a one-line fragment contains a variable
-          // assignment.
-          //
-          if (refl.find ('\n') == string::npos)
-            dr << info << "reflect variable: " << trim (string (refl));
-          else
-            dr << info << "reflect clause:\n"
-               << trim_right (string (refl));
-
-          // For external packages we have the manifest so print the location
-          // of the depends value in questions.
-          //
-          if (rs.out_eq_src ())
-            dr << info << "in depends manifest value of package "
-               << package.name;
-          else
-            depends_location (dr,
-                              rs.src_path () / manifest_file,
-                              depends_index);
-        });
-
-      // Note: a lot of this code is inspired by the config module.
-      //
-
-      // Collect all the set config.<name>.* variables on the first pass and
-      // filter out unchanged on the second.
-      //
-      auto& vp (rs.var_pool ());
-      const string& ns (var_prefix_);
-
-      struct value_data
-      {
-        const value* val;
-        size_t       ver;
-      };
-
-      // @@ Maybe redo as small_vector?
-      //
-      map<const variable*, value_data> vars;
-
-      auto process = [&rs, &ns, &vars] (bool collect)
-      {
-        for (auto p (rs.vars.lookup_namespace (ns));
-             p.first != p.second;
-             ++p.first)
-        {
-          const variable& var (p.first->first);
-
-          // This can be one of the overrides (__override, __prefix, etc),
-          // which we skip.
-          //
-          if (var.override ())
-            continue;
-
-          // What happens to version if overriden? A: appears to be still
-          // incremented!
-          //
-          const variable_map::value_data& val (p.first->second);
-
-          if (collect)
-          {
-            vars.emplace (&var, value_data {nullptr, val.version});
-          }
-          else
-          {
-            auto i (vars.find (&var));
-
-            if (i != vars.end ())
-            {
-              if (i->second.ver == val.version)
-                vars.erase (i); // Unchanged.
-              else
-                i->second.val = &val;
-            }
-            else
-              vars.emplace (&var, value_data {&val, 0});
-          }
-        }
-      };
-
-      process (true);
-
-      lexer l (is, in, il /* start line */);
-      buildfile_parser p (rs.ctx,
-                          dependency_var_prefixes_,
-                          dependency_var_prefixes_pending);
-      p.parse_buildfile (l, &rs, rs);
-
-      process (false);
-
-      // Add to the map the reflect variables collected previously. Note that
-      // we can re-purpose the override since we re-populate it.
-      //
-      for (string& n: reflect_vars_)
-      {
-        // Transform `name=value` to just `name`.
-        //
-        {
-          size_t p (n.find ('='));
-          assert (p != string::npos); // We've serialized it so must be there.
-          n.resize (p);
-        }
-
-        auto p (vars.emplace (&vp.insert (move (n)), value_data {nullptr, 0}));
-
-        if (p.second)
-        {
-          // The value got to be there since it's set by the accumulated
-          // fragment we've evaluated before root.build.
-          //
-          p.first->second.val = rs.vars[p.first->first].value;
-        }
-      }
-
-      // Re-populate everything from the map.
-      //
-      reflect_vars_.clear ();
-      reflect_frag_.clear ();
-
-      // Collect the config.<name>.* variables that were changed by this
-      // and previous reflect clauses.
-      //
-      // Specifically, we update both the accumulated fragment to be evaluated
-      // before root.build on the next load (if any) as well as the merged
-      // configuration variable overrides to be passed during the package
-      // configuration. Doing both of these now (even though one of them won't
-      // be needed) allows us to immediately drop the build context and
-      // release its memory. It also makes the implementation a bit simpler
-      // (see, for example, the copy constructor).
-      //
-      names storage;
-      for (const auto& p: vars)
-      {
-        const variable& var (*p.first);
-        const value& val (*p.second.val);
-
-        // For the accumulated fragment we always save the original and let
-        // the standard overriding take its course.
-        //
-        serialize_buildfile (reflect_frag_, var.name, val, storage);
-
-        // Note: this is currently disabled and is likely very outdated.
-        //
-        // For the accumulated overrides we have to merge user config_vars_
-        // with reflect values. Essentially, we have three possibilities:
-        //
-        // 1. There is no corresponding reflect value for a user value. In
-        //    this case we just copy over the user value.
-        //
-        // 2. There is no corresponding user value for a reflect value. In
-        //    this case we just copy over the reflect value.
-        //
-        // 3. There are both reflect and user values. In this case we replace
-        //    the user value with the final (overriden) value using plain
-        //    assignment (`=`). We do it this way to cover append overrides,
-        //    for example:
-        //
-        //    config.hello.backend = foo  # reflect
-        //    config.hello.backend += bar # user
-        //
-
-        // @@ Can't we redo it via origin() call like in other places?
-        //
-        pair<lookup, size_t> org {lookup {val, var, rs.vars}, 1 /* depth */};
-        pair<lookup, size_t> ovr;
-
-        if (var.overrides == nullptr)
-          ovr = org; // Case #2.
-        else
-        {
-          // NOTE: see also below if enabling this.
-          //
-#if 0
-          // Case #3.
-          //
-          // The override can come from two places: config_vars_ or one of the
-          // "global" sources (environment variable, default options file; see
-          // load() for details). The latter can only be a global override and
-          // can be found (together with global overrides from config_vars_)
-          // in context::global_var_overrides.
-          //
-          // It feels like mixing global overrides and reflect is misguided:
-          // we probably don't want to rewrite it with a global override (per
-          // case #3 above) since it will apply globally. So let's diagnose it
-          // for now.
-          //
-          {
-            const strings& ovs (ctx_->global_var_overrides);
-            auto i (find_if (ovs.begin (), ovs.end (),
-                             [&var] (const string& o)
-                             {
-                               // TODO: extracting name is not easy.
-                             }));
-
-            if (i != ovs.end ())
-            {
-              fail << "global override for reflect clause variable " << var <<
-                info << "global override: " << *i;
-            }
-          }
-
-          // Ok, this override must be present in config_vars_.
-          //
-          // @@ Extracting the name from config_vars_ and similar is not easy:
-          //    they are buildfile fragments and context actually parses them.
-          //
-          // @@ What if we have multiple overrides?
-          //
-          // @@ What if it's some scoped override or some such (e.g., all
-          //    these .../x=y, etc).
-          //
-          // @@ Does anything change if we have an override but it does not
-          //    apply (i.e., ovr == org && var.overrides != nullptr)?
-          //
-          // @@ Perhaps a sensible approach is to start relaxing/allowing
-          //    this for specific, sensible cases (e.g., single unqualified
-          //    override)?
-          //
-          // What would be the plausible scenarios for an override?
-          //
-          // 1. Append override that adds some backend or some such to the
-          //    reflect value.
-          //
-          // 2. A reflect may enable a feature based on the dependency
-          //    alternative selected (e.g., I see we are using Qt6 so we might
-          //    as well enable feature X). The user may want do disable it
-          //    with an override.
-          //
-          ovr = rs.lookup_override (var, org);
-#else
-          // @@ TODO: probably also depends, not just user.
-          // @@ TODO: maybe we should allow if the same value (say a bool
-          //          feature set to true but both)?!
-
-          fail << "command line override of reflect clause variable " << var
-               << endf;
-#endif
-        }
-
-        reflect_vars_.push_back (
-          serialize_cmdline (var.name, *ovr.first, storage));
-      }
-
-#if 0
-      // TODO: copy over config_vars_ that are not in the map (case #1).
-#endif
-
-      // Drop the build system state since it needs reloading (some computed
-      // values in root.build may depend on the new configuration values).
-      //
-      ctx_ = nullptr;
-    }
-    catch (const build2::failed&)
-    {
-      throw failed (); // Assume the diagnostics has already been issued.
-    }
-  }
-
-#endif
-
   bool package_skeleton::
   evaluate_prefer_accept (const dependency_configurations& cfgs,
                           const string& prefer,