[zypp-devel] Re: [zypp-commit] r10919 - /trunk/libzypp/zypp/parser/RepoindexFileReader.cc
Hi Jano, [sorry you get this twice] On Wed, 3 Sep 2008, jkupec@svn.opensuse.org wrote:
Log: - skip repositories with non-matching target distro when reading repoindex
@@ -98,6 +103,19 @@ if ( reader_r->name() == "repo" ) { XmlString s; + + // skip repositories with unknown or not matching target distribution + s = reader_r->getAttribute("distro_target"); + if (!s.get() || _target_distro != s.asString()) + { + MIL + << "Skipping repository meant for '" + << (s.get() ? s.asString() : "(not specified)") + << "' distribution (current distro is '" << _target_distro << "')." + << endl; + return true; + } +
If I understand the code correctly I don't think this is a good idea. If the repofile doesn't have any entry for "distro_target" it will be skipped. I rather would think that this is supposed to mean "don't care". Even _if_ it has a target-distro set (that's a new concept anyway, right?) why should we reject to use it? E.g. if it's completely self-contained (in the sense of only having external dependencies on, say, libc), like I would expect with some vendor repos, I can and want to use it on all distros. Also, how is the name matched? Above it's a string comparison. But 11.1 and SLES11 will be roughly the same, for instance. To me it sounds more a feature needlessly annoying users. What's the purpose of this? Ciao, Michael. -- To unsubscribe, e-mail: zypp-devel+unsubscribe@opensuse.org For additional commands, e-mail: zypp-devel+help@opensuse.org
On Wed, Sep 03, Michael Matz wrote:
If I understand the code correctly I don't think this is a good idea. If the repofile doesn't have any entry for "distro_target" it will be skipped. I rather would think that this is supposed to mean "don't care".
Even _if_ it has a target-distro set (that's a new concept anyway, right?) why should we reject to use it? E.g. if it's completely self-contained (in the sense of only having external dependencies on, say, libc), like I would expect with some vendor repos, I can and want to use it on all distros.
Also, how is the name matched? Above it's a string comparison. But 11.1 and SLES11 will be roughly the same, for instance.
To me it sounds more a feature needlessly annoying users. What's the purpose of this?
Furthermore the location of this test in zypp/parser/RepoindexFileReader questionable. Parsing a file should not require an initialized target. Your code will throw e.g in installation mode because there is no target. You also restrict us to parse only matching repoinfos. That's %^&*(. Parsing a file is not the same as using a repo. This test is not a parsers job (->RepoManager). And the default action for repositories with unknown or not matching target distribution should probably be configurable. Why not asking the user, and if no user available, use a config default. -- cu, Michael Andres +------------------------------------------------------------------+ Key fingerprint = 2DFA 5D73 18B1 E7EF A862 27AC 3FB8 9E3A 27C6 B0E4 +------------------------------------------------------------------+ Michael Andres YaST Development ma@novell.com SUSE LINUX Products GmbH, GF: Markus Rex, HRB 16746 (AG Nuernberg) Maxfeldstrasse 5, D-90409 Nuernberg, Germany, ++49 (0)911 - 740 53-0 +------------------------------------------------------------------+ -- To unsubscribe, e-mail: zypp-devel+unsubscribe@opensuse.org For additional commands, e-mail: zypp-devel+help@opensuse.org
Parsing a file should not require an initialized target. Your code will throw e.g in installation mode because there is no target.
I realized that, already fixing.
You also restrict us to parse only matching repoinfos. That's %^&*(. Parsing a file is not the same as using a repo.
This test is not a parsers job (->RepoManager).
agreed, i'll move it there.
And the default action for repositories with unknown or not matching target distribution should probably be configurable. Why not asking the user, and if no user available, use a config default.
yes, i'm already doing this. I should not have committed so soon :O) ----------------------- Michael Matz wrote:
Hi Jano,
On Wed, 3 Sep 2008, jkupec@svn.opensuse.org wrote:
Log: - skip repositories with non-matching target distro when reading repoindex
@@ -98,6 +103,19 @@ if ( reader_r->name() == "repo" ) { XmlString s; + + // skip repositories with unknown or not matching target distribution + s = reader_r->getAttribute("distro_target"); + if (!s.get() || _target_distro != s.asString()) + { + MIL + << "Skipping repository meant for '" + << (s.get() ? s.asString() : "(not specified)") + << "' distribution (current distro is '" << _target_distro << "')." + << endl; + return true; + } +
If I understand the code correctly I don't think this is a good idea. If the repofile doesn't have any entry for "distro_target" it will be
not repofile! repoindex.xml (service). This is RepoindexFileReader, not RepoFileReader, just in case... :O)
skipped. I rather would think that this is supposed to mean "don't care".
Hmmm.. maybe you're right.... if the distro_target will not be specified, we should add the repo regardless on the target distro.
Even _if_ it has a target-distro set (that's a new concept anyway, right?)
Not really, ZLM has been already using it in code10, we are just reimplementing the NU services support in zypp.
why should we reject to use it? E.g. if it's completely self-contained (in the sense of only having external dependencies on, say, libc), like I would expect with some vendor repos, I can and want to use it on all distros.
Right, but in that case the repo should not have target_distro set in the repoindex.xml.
Also, how is the name matched? Above it's a string comparison. But 11.1 and SLES11 will be roughly the same, for instance.
To me it sounds more a feature needlessly annoying users. What's the
It has to be an exact string match. The distro ID written in /etc/product.d/baseproduct will be one of the standard IDs. We will have a list of possible values soon. purpose of this? Imagine a repoindex with a few repos, each for several distros and each architecture. You only want foo-i386, so this is to filter out those not-matching. cheers, jano -- To unsubscribe, e-mail: zypp-devel+unsubscribe@opensuse.org For additional commands, e-mail: zypp-devel+help@opensuse.org
Hi, On Wed, 3 Sep 2008, Jan Kupec wrote:
skipped. I rather would think that this is supposed to mean "don't care".
Hmmm.. maybe you're right.... if the distro_target will not be specified, we should add the repo regardless on the target distro.
Well, there has to be a way to use repos for multiple target distros. E.g. when there is no real target distro at all (e.g. because it's a mixture).
why should we reject to use it? E.g. if it's completely self-contained (in the sense of only having external dependencies on, say, libc), like I would expect with some vendor repos, I can and want to use it on all distros.
Right, but in that case the repo should not have target_distro set in the repoindex.xml.
Okay, then we are in agreement, that this is the "don't care" case. Ciao, Michael. -- To unsubscribe, e-mail: zypp-devel+unsubscribe@opensuse.org For additional commands, e-mail: zypp-devel+help@opensuse.org
participants (3)
-
Jan Kupec
-
Michael Andres
-
Michael Matz