Bug#594435: Quoted strings in pin specifications do not work

2010-08-30 Thread David Kalnischkies
Hi!

2010/8/26 Julian Andres Klode j...@debian.org:
 On Do, 2010-08-26 at 01:38 +0100, Ben Hutchings wrote:
 The *only* example of pinning by origin in apt_preferences(5) is:

That was fixed in version 0.7.26~exp3 and is part of 0.8.0 which
will be in a few days in testing hopefully.


 I personally do not want quoting here, as it just complicates parsing
 and breaks backwards compatibility. The documentation could state
 explicitly that quotes are not supported; but this is purely a
 documentation issue, and thus minor (it may also break translations of
 the manpage).

I don't get the complicated parser argument, how could a single char
increase the complexity of a parser - especially as the empty string
needs to be represented somehow anyway…
And compatibility? Its long ago that i saw a hostname starting with …
Or do we need to maintain compatibilty with this bugs as users have non
working rules in their preferences file because of this bug?

I personally don't want to complicated it for a human being if a
machine could do the hard work - at least in my understand we created
machines to do the hard and boring work for us in the first place, so why
enforce a silly no quotes allowed - but needed if origin is empty rule?


Anyway, the fun fact is that the manpage is wrong:
First of all  doesn't work as expected, especially not if the local archive
is not flat. Further more i made the mistake to document the origin now
by writing ftp.de.debian.org which is obviously also wrong.

Thankfully, we have two simple options to fix it:
1. Fix  and remove the quotes around the hostname by fiddling with the po's
2. Fix  and allow the quotes around the hostname

Attached is the overly complicated patch to fix it by implementing option 2 -
option 1 would only differ a bit in the first if-else block and would need
a bit too much handwork on the po files for my taste…


Best regards

David Kalnischkies
=== modified file 'apt-pkg/versionmatch.cc'
--- apt-pkg/versionmatch.cc	2010-06-28 15:38:08 +
+++ apt-pkg/versionmatch.cc	2010-08-30 09:33:13 +
@@ -118,7 +118,10 @@

if (Type == Origin)
{
-  OrSite = Data;
+  if (Data[0] == ''  Data.end()[-1] == '')
+	 OrSite = Data.substr(1, Data.length() - 2);
+  else
+	 OrSite = Data;
   return;
}   
 }
@@ -259,10 +262,10 @@
if (Type == Origin)
{
   if (OrSite.empty() == false) {
-	 if (File-Site == 0 || !ExpressionMatches(OrSite, File.Site()))
+	 if (File-Site == 0)
 	return false;
   } else // so we are talking about file:// or status file
-	 if (strcmp(File.Site(),) == 0  File-Archive != 0) // skip the status file
+	 if (strcmp(File.Site(),) == 0  File-Archive != 0  strcmp(File.Archive(),now) == 0) // skip the status file
 	return false;
   return (ExpressionMatches(OrSite, File.Site())); /* both strings match */
}



Bug#594435: Quoted strings in pin specifications do not work

2010-08-30 Thread Ben Hutchings
On Mon, 2010-08-30 at 11:34 +0200, David Kalnischkies wrote:
 === modified file 'apt-pkg/versionmatch.cc'
 --- apt-pkg/versionmatch.cc 2010-06-28 15:38:08 +
 +++ apt-pkg/versionmatch.cc 2010-08-30 09:33:13 +
 @@ -118,7 +118,10 @@
 
 if (Type == Origin)
 {
 -  OrSite = Data;
 +  if (Data[0] == ''  Data.end()[-1] == '')
 +OrSite = Data.substr(1, Data.length() - 2);
 +  else
 +OrSite = Data;
return;
 }   
  }

Is Data guaranteed to be non-empty here?  If not, you should check that
first.

Ben.

-- 
Ben Hutchings
Once a job is fouled up, anything done to improve it makes it worse.


signature.asc
Description: This is a digitally signed message part


Bug#594435: Quoted strings in pin specifications do not work

2010-08-30 Thread Julian Andres Klode
On Mo, 2010-08-30 at 11:34 +0200, David Kalnischkies wrote:
 Hi!
 
 2010/8/26 Julian Andres Klode j...@debian.org:
  On Do, 2010-08-26 at 01:38 +0100, Ben Hutchings wrote:
  The *only* example of pinning by origin in apt_preferences(5) is:
 
 That was fixed in version 0.7.26~exp3 and is part of 0.8.0 which
 will be in a few days in testing hopefully.
 
 
  I personally do not want quoting here, as it just complicates parsing
  and breaks backwards compatibility. The documentation could state
  explicitly that quotes are not supported; but this is purely a
  documentation issue, and thus minor (it may also break translations of
  the manpage).
 
 I don't get the complicated parser argument, how could a single char
 increase the complexity of a parser - especially as the empty string
 needs to be represented somehow anyway…
 And compatibility? Its long ago that i saw a hostname starting with …
 Or do we need to maintain compatibilty with this bugs as users have non
 working rules in their preferences file because of this bug?
Well, for origin Pins alone this does not apply. I was talking about
allowing quoted strings everywhere in preferences files. And yes, a
hostname can not contain quotes at all, at least if you want to follow
RFC 3986.

Of course, if you only add quotes to origins, you break nothing. But as
a hostname can not contain spaces, having quotes there is useless; and
it still gives you the assumption that you can use quotes every.

 
 I personally don't want to complicated it for a human being if a
 machine could do the hard work - at least in my understand we created
 machines to do the hard and boring work for us in the first place, so why
 enforce a silly no quotes allowed - but needed if origin is empty rule?
 
 
 Anyway, the fun fact is that the manpage is wrong:
 First of all  doesn't work as expected, especially not if the local archive
 is not flat. Further more i made the mistake to document the origin now
 by writing ftp.de.debian.org which is obviously also wrong.
 
 Thankfully, we have two simple options to fix it:
 1. Fix  and remove the quotes around the hostname by fiddling with the po's
 2. Fix  and allow the quotes around the hostname
 
 Attached is the overly complicated patch to fix it by implementing option 2 -
 option 1 would only differ a bit in the first if-else block and would need
 a bit too much handwork on the po files for my taste…
Any reason why you break/remove pattern matching in your patch? I think
Pin: origin *debian.org
Pin: origin /.*debian.org/
can be really useful (that's why I wrote it).

-- 
Julian Andres Klode  - Debian Developer, Ubuntu Member

See http://wiki.debian.org/JulianAndresKlode and http://jak-linux.org/.





-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#594435: Quoted strings in pin specifications do not work

2010-08-30 Thread David Kalnischkies
 Is Data guaranteed to be non-empty here?  If not, you should check that
 first.

It is as the first real line of the method checks for Data.length()  1
and otherwise exits, so Data is at least one char long.
You can still do a silly Pin: origin 
which should maybe be catched as not allowed to match an
empty string, but even if detected it can't be expressed with a
error message as the parser seems to be too dumb for it…


2010/8/30 Julian Andres Klode j...@debian.org:
 On Mo, 2010-08-30 at 11:34 +0200, David Kalnischkies wrote:
  I personally do not want quoting here, as it just complicates parsing
  and breaks backwards compatibility. The documentation could state
  explicitly that quotes are not supported; but this is purely a
  documentation issue, and thus minor (it may also break translations of
  the manpage).

 I don't get the complicated parser argument, how could a single char
 increase the complexity of a parser - especially as the empty string
 needs to be represented somehow anyway…
 And compatibility? Its long ago that i saw a hostname starting with …
 Or do we need to maintain compatibilty with this bugs as users have non
 working rules in their preferences file because of this bug?
 Well, for origin Pins alone this does not apply. I was talking about
 allowing quoted strings everywhere in preferences files. And yes, a
 hostname can not contain quotes at all, at least if you want to follow
 RFC 3986.

 Of course, if you only add quotes to origins, you break nothing. But as
 a hostname can not contain spaces, having quotes there is useless; and
 it still gives you the assumption that you can use quotes every.

but a hostname can be empty in which case the preferences manpage says
Pin: origin  will match.
Pin: release a= on the other hand will not work and is also not
advertised this way in the manpage nor in the apt-cache policy output.

I think it is just tempting to write hostname if you have an example in
that way, even if the hostname in this example is empty.
The rule hostname or empty feels more natural than hostname or .
Especially if no error is given for wrong syntax…

The space argument is in so far complicated as Label and Co could include
spaces and Pin: release l=some label is not supported in any way…

 Attached is the overly complicated patch to fix it by implementing option 2 -
 option 1 would only differ a bit in the first if-else block and would need
 a bit too much handwork on the po files for my taste…
 Any reason why you break/remove pattern matching in your patch? I think
        Pin: origin *debian.org
        Pin: origin /.*debian.org/
 can be really useful (that's why I wrote it).

In which way is it broken?
I just removed the first invocation as matching doesn't need to be checked
twice if the entry matches, thats all, the pattern magic still happens in the
return line…


Best regards

David Kalnischkies



--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#594435: Quoted strings in pin specifications do not work

2010-08-26 Thread Julian Andres Klode
severity 594435 minor
thanks

On Do, 2010-08-26 at 01:38 +0100, Ben Hutchings wrote:
 Package: apt
 Version: 0.7.25.3
 Severity: important
 
 The *only* example of pinning by origin in apt_preferences(5) is:
 
Package: *
Pin: origin 
Pin-Priority: 999
 
 This implies that strings can be double-quoted and that an empty
 string must be.  In fact, double-quoted strings do not match their
 contents; the quote characters are presumably treated literally.
Not really. It only applies to empty strings which can not be
represented otherwise in a meaningful way; for example, specifying no
value would be invalid (or should be, I did not try it).

I personally do not want quoting here, as it just complicates parsing
and breaks backwards compatibility. The documentation could state
explicitly that quotes are not supported; but this is purely a
documentation issue, and thus minor (it may also break translations of
the manpage).

-- 
Julian Andres Klode  - Debian Developer, Ubuntu Member

See http://wiki.debian.org/JulianAndresKlode and http://jak-linux.org/.





-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#594435: Quoted strings in pin specifications do not work

2010-08-25 Thread Ben Hutchings
Package: apt
Version: 0.7.25.3
Severity: important

The *only* example of pinning by origin in apt_preferences(5) is:

   Package: *
   Pin: origin 
   Pin-Priority: 999

This implies that strings can be double-quoted and that an empty
string must be.  In fact, double-quoted strings do not match their
contents; the quote characters are presumably treated literally.

Ben.

-- Package-specific info:

-- (/etc/apt/preferences present, but not submitted) --


-- (/etc/apt/sources.list present, but not submitted) --


-- System Information:
Debian Release: squeeze/sid
  APT prefers proposed-updates
  APT policy: (500, 'proposed-updates'), (500, 'unstable'), (500, 'stable'), 
(1, 'experimental')
Architecture: i386 (x86_64)

Kernel: Linux 2.6.32-5-amd64 (SMP w/2 CPU cores)
Locale: LANG=en_GB.UTF-8, LC_CTYPE=en_GB.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash

Versions of packages apt depends on:
ii  debian-archive-keyring2010.08.15 GnuPG archive keys of the Debian a
ii  libc6 2.11.2-2   Embedded GNU C Library: Shared lib
ii  libgcc1   1:4.4.4-9  GCC support library
ii  libstdc++64.4.4-9The GNU Standard C++ Library v3

apt recommends no packages.

Versions of packages apt suggests:
pn  apt-doc   none (no description available)
ii  aptitude  0.6.3-3terminal-based package manager (te
ii  bzip2 1.0.5-4high-quality block-sorting file co
ii  dpkg-dev  1.15.8.4   Debian package development tools
ii  lzma  4.43-14Compression method of 7z format in
ii  python-apt0.7.96.1   Python interface to libapt-pkg
ii  synaptic  0.70~pre1  Graphical package manager

-- no debconf information



-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org