On Tue, Feb 11, 2014 at 12:39:29PM +0100, Johannes Schauer wrote:
> Quoting Julian Andres Klode (2013-01-30 12:57:18)
> > DEP-11 plans to make '<' and '>' basically part of the package name and use
> > that for more complex provides; not knowing the use in multi-arch (I forgot
> > it). So the extended architecture syntax seems better in my opinion; as it
> > does not conflict with that stuff.
>
> for the better or worse, the <!profile.stage1> syntax was accepted in dpkg
> 1.17.2. Looking at http://dep.debian.net/deps/ it seems that DEP-11 has been
> removed?
On the index it is still listed as candidate. I think I read something
about the whole dep infrastructure being broken at the moment. dunno
where… anyway not really the topic:
> I prepared a patch which adds the current implementation of build profiles in
> dpkg to apt.
So, we talk about the commit 7662e0937bb064a0754d12605d80a96a17e2aadf
in dpkg, right? I haven't kept close tabs on the discussion…
(and haven't done much expect looking for the commit now either)
> + std::vector<string> profiles;
> +
> + // accept non-list order as override setting
> + string const overrideProfiles = _config->Find("APT::Build-Profiles", "");
> +
> + if (overrideProfiles.empty() == false)
> + profiles.push_back(overrideProfiles);
> +
> + std::vector<string> p = _config->FindVector("APT::Build-Profiles");
> + profiles.insert(profiles.end(), p.begin(), p.end());
dpkg allows in its commandline flag a comma-separated list.
I guess it would be good to support it here as well then in the
"override". It also supports an environment variable:
Is it by design that apt tools should not?
Also, the comment says "as override setting" but it isn't overriding
at all, it is just another input.
> + std::string prefix = "profile.";
> + // only support for "profile" prefix
> + if (profile.size() <= prefix.size() ||
> + profile.substr(0, prefix.size()) != prefix) {
> + // wrong prefix, so fast-forward to the end of the wildcards
> + for (; End != Stop && *End != '>'; ++End);
Really? Wouldn't it make more sense to just ignore the "thing" here
(aka: positive always true, negative always false) rather than skipping
right to the end? It will potentially be very hard to keep everyone
using <profile.stage1 awesome.debian> instead of the other way around if
we ever introduce an "awesome.debian" 'thing'.
(It can't be ProfileFlags as these are only "profile.*" flags in my
reading. There should be a name for this new <> stuff and it should be
used instead of the name "ProfileFlags" here.)
Either way, a testcase documenting this skipping would be good.
> diff --git a/apt-pkg/deb/deblistparser.h b/apt-pkg/deb/deblistparser.h
> index 386d291..fff4d00 100644
> --- a/apt-pkg/deb/deblistparser.h
> +++ b/apt-pkg/deb/deblistparser.h
> @@ -76,7 +76,8 @@ class debListParser : public pkgCacheGenerator::ListParser
> static const char *ParseDepends(const char *Start,const char *Stop,
> std::string &Package,std::string &Ver,unsigned int &Op,
> bool const &ParseArchFlags = false,
> - bool const &StripMultiArch = true);
> + bool const &StripMultiArch = true,
> + bool const &ParseProfileFlags = false);
To remain binary compatible, please make this a proper overload rather
than adding just a new parameter – and lets just hope nobody has &func on
this one…
http://techbase.kde.org/Policies/Binary_Compatibility_Issues_With_C++#The_Do.27s_and_Don.27ts
tends to be a handy cheatsheet for these kind of considerations.
> diff --git a/doc/apt.conf.5.xml b/doc/apt.conf.5.xml
>
> + <varlistentry><term><option>Build-Profiles</option></term>
> + <listitem><para>
> + All enabled build profiles for source package builds. By default this
> + list is empty.
> + </para></listitem>
> + </varlistentry>
> +
This is… short. Do we have some document we could link from here, like a
section in a dpkg manpage which is supposed to give a few more details?
Best regards
David Kalnischkies
Attachment:
signature.asc
Description: Digital signature