[PATCH] regarding bugs #15 and #63
![](https://secure.gravatar.com/avatar/6d9cfa47b02fe605d52f16450c1984ee.jpg?s=120&d=mm&r=g)
Hi all, First of all, I'm sorry about not using bug tracker, but I have the reason: I suppose that this patch will cause some sort of debates, and mailing list fits it best. And I hate www technique for conversation purposes. ;-) Second - pkgutils have [security] flaw. One package can seriously damage whole system without any warnings. Is conflicts catching should be done in pkgutils or not is arguable, but we have some sort of it already, thus we should make it more consistent. This patch fixes bugs mentioned in subject. The patch is about the same size as summary sizes of all *.cc files. But! Please, don't be scared, there are really small amount of logic changes. Most line-consuming changes are: new types, old types typedef'ed, long functions splitting, new debugging code, pkgadd.cc changes regarding indentation and long lines. The real change is that pkgutils now reclines not only on filenames but on `file_t' type, which is pair: filename and `fileinfo_t'. fileinfo_t is a struct that contains mode, uid and gid fields (others can be added); Another new type is fileconflict_t, which is pair of file_t and conflict_t. conflict_t may be: none, database, filesystem and mode. Plural forms of above types also introduced. There are ~280 lines added, but I repeat: nothing grandeur happened. I'm attaching the patch and test-case Pkgfile. pkgadd's output will be: # pkgutils/pkgadd foo#1.0-1.pkg.tar.gz Following files conflicts with database records: usr/sbin/zic Following files conflicts with filesystem files: dev/tty0 Following files conflicts by mode or ownership: drwxr-xrwx uid: 0 gid: 0 bin/ drwxr-xrwx uid: 0 gid: 0 sbin/ pkgadd: use -f to ignore and overwrite If there are permissions changes during update of the package, pkgadd will complain about it. I'm (as a user) believe that I should be warned about permission changes. Though, it's trivial to disable this feature, if anyone will insist. Hope somebody from core development team will able to review and apply it. I'm ready to answer any questions regarding changes. Good luck, -- Anton (irc: bd2) p.s. Also, I hope I'm not introduced new bugs. If so, please tell me.
![](https://secure.gravatar.com/avatar/4e374bb9f03cbbca5d9541a8bf8ec8bf.jpg?s=120&d=mm&r=g)
Hi Anton, On Tue, Jul 11, 2006 at 03:40:08 +0400, Anton wrote:
Hi all, [...] Second - pkgutils have [security] flaw. One package can seriously damage whole system without any warnings. Is conflicts catching should be done in pkgutils or not is arguable, but we have some sort of it already, thus we should make it more consistent. I assume you're refering to #63 here, right? The symlink bug #15 doesn't qualify as a security bug in my opinion. Also, does it really fix #15, i.e. follow symlinks? I might have looked over it too quickly, but I didn't find any indication.
Have you though about other cases which could be potentially annoying to people, e.g. when they manually adjust permissions? Other than that, if the potential "false positives" aren't too bad (analysis needed), it seems like a feature worth having and even adding a couple of lines. I haven't looked at the output it provides in case of a conflict, we should make sure it's clear to the user.
This patch fixes bugs mentioned in subject.
The patch is about the same size as summary sizes of all *.cc files. But! Please, don't be scared, there are really small amount of logic changes. Then it should be a small patch :-)
Most line-consuming changes are: new types, old types typedef'ed, long functions splitting, new debugging code, pkgadd.cc changes regarding indentation and long lines. I'd appreciate if you could split it up. I've reviewed it quickly, and while I like the ideas, some of the refactorings seem to be personal preference: the indentation changes, typedefs etc. and while this doesn't mean that those changes should be rejected, it seems more sensible to look at those changes isolated, and independent from the bug fix. If you could provide individual patches (debug changes, indentation, nesting removal etc.) we can decide on a per patch basis whether we agree there, and eventually come up with a merged version we can all live with (hopefully), and which supports mode-conflicts.
As a final remark, I find your use of typedefs slightly excessive here; I usually prefer to write the real type instead of looking up typedefs, especially as there are so many typedefs with similar names: file_t, files_t, filenames_t, fileconflict_t, fileconflicts_t I haven't really thought a lot about it yet, it just kinda "smells"; maybe some naming changes might help already. Other than that, the patch is very clean, c++ish, and I look forward to discuss the changes in more detail. Thanks for your work so far. Regards, Johannes -- Johannes Winkelmann mailto:jw@smts.ch Zurich, Switzerland http://jw.smts.ch
![](https://secure.gravatar.com/avatar/6d9cfa47b02fe605d52f16450c1984ee.jpg?s=120&d=mm&r=g)
Hi Johannes, On Tue, Jul 11, 2006 at 10:41:44AM +0200, Johannes Winkelmann wrote:
Second - pkgutils have [security] flaw. One package can seriously damage whole system without any warnings. Is conflicts catching should be done in pkgutils or not is arguable, but we have some sort of it already, thus we should make it more consistent. I assume you're refering to #63 here, right? The symlink bug #15 doesn't qualify as a security bug in my opinion.
Right.
Also, does it really fix #15, i.e. follow symlinks? I might have looked over it too quickly, but I didn't find any indication.
I can't reproduce bug#15's test case, thus I assume it is fixed. If there another test case exist, I can test it and try to fix.
Have you though about other cases which could be potentially annoying to people, e.g. when they manually adjust permissions?
Maybe I should introduce DONTCARE_{DIR,FILE}_PERMISSIONS config entry later. But really I can't imagine the case when someone have to change permissions on say /var or /bin/ls. And if they doing it, with current pkgutils they are risking to lost their changes on the package update.
Other than that, if the potential "false positives" aren't too bad (analysis needed), it seems like a feature worth having and even adding a couple of lines. I haven't looked at the output it provides in case of a conflict, we should make sure it's clear to the user.
This patch fixes bugs mentioned in subject.
The patch is about the same size as summary sizes of all *.cc files. But! Please, don't be scared, there are really small amount of logic changes. Then it should be a small patch :-)
Mmm.. no. :-) That is a point of refactoring: you change small piece of code, and whole code needs to be changed here and there, because of initial change. Like an avalanche. If it was trivial change, yes it will be a small patch. But this is not so trivial change, but in the same it do not touch algorithms logic.
Most line-consuming changes are: new types, old types typedef'ed, long functions splitting, new debugging code, pkgadd.cc changes regarding indentation and long lines. I'd appreciate if you could split it up.
It's hard to do, because things is somewhat twisted. If it's really important you to read changes splitted, I will try.
I've reviewed it quickly, and while I like the ideas, some of the refactorings seem to be personal preference: the indentation changes, typedefs etc. and while this doesn't mean that those changes should be rejected, it seems more sensible to look at those changes isolated, and independent from the bug fix. If you could provide individual patches (debug changes, indentation, nesting removal etc.) we can decide on a per patch basis whether we agree there, and eventually come up with a merged version we can all live with (hopefully), and which supports mode-conflicts.
As a final remark, I find your use of typedefs slightly excessive here; I usually prefer to write the real type instead of looking up typedefs, especially as there are so many typedefs with similar names: file_t, files_t, filenames_t, fileconflict_t, fileconflicts_t I haven't really thought a lot about it yet, it just kinda "smells"; maybe some naming changes might help already.
Yes, I use and like typedefs. So do Per as far as I can see, because he introduced packages_t in pkgutil.h, but for unknown reason he do not use it in many places of code. I suppose he just don't find enough time to switch all set<> to proper types. This what I've done. For the most of time stl algorithms do not care which type you use, be it vector, deque or set. It's all containers, which is nameless, and it's up to programmer to name it. The code will rather ugly and obscure without typedefs. That is why typedef invented, btw. ;-)
Other than that, the patch is very clean, c++ish, and I look forward to discuss the changes in more detail.
Thanks.
Thanks for your work so far. Regards, Johannes
-- Anton (irc: bd2)
![](https://secure.gravatar.com/avatar/4e374bb9f03cbbca5d9541a8bf8ec8bf.jpg?s=120&d=mm&r=g)
Hi, On Tue, Jul 11, 2006 at 19:24:48 +0400, Anton wrote: > Hi Johannes, > > On Tue, Jul 11, 2006 at 10:41:44AM +0200, Johannes Winkelmann wrote: [...] > > Have you though about other cases which could be potentially annoying to > > people, e.g. when they manually adjust permissions? > > Maybe I should introduce DONTCARE_{DIR,FILE}_PERMISSIONS config entry > later. But really I can't imagine the case when someone have to change > permissions on say /var or /bin/ls. And if they doing it, with current > pkgutils they are risking to lost their changes on the package update. Having thought about it for a while I think there's a whole family of mode changes which would cause an unwanted conflict: - a port changing one of its binaries to be suid root (or not suid root anymore), or any other mode change - a port (e.g. a daemon) changing to be run as nobody from root, thus requiring its data file to be owned by nobody:nobody - same problem: a port starting to use a particular group (think: video, audio etc.) which previously used 'root' - human errors where a directory wasn't writable or a file not executable, and fixed by the maintainer by changing a mode/ownership of some files or directories Especially the last is likely to happen to us also in the future, and the current version of pkgutils handles those wanted and intentional mode/ownership changes gracefully (which is a nice way to say it, it obviously just ignores it). Considering that, I'm not sure if the conflict mode should be on by default. It definitely makes sense for maintainers to avoid such mode changes by mistake, and it protects users from malicious ports, however I'm not convinced yet whether protecting from the theoretical thread is worth removing the functionality to make fixes to mode/ownership without special interaction. > > > This patch fixes bugs mentioned in subject. > > > > > > The patch is about the same size as summary sizes of all *.cc files. > > > But! Please, don't be scared, there are really small amount of logic > > > changes. > > Then it should be a small patch :-) > > Mmm.. no. :-) That is a point of refactoring: [...] Well, I meant the bugfix which could be a small(er) patch when separated from the refactorings. > > > Most line-consuming changes are: new types, old types typedef'ed, > > > long functions splitting, new debugging code, pkgadd.cc changes regarding > > > indentation and long lines. > > I'd appreciate if you could split it up. > > It's hard to do, because things is somewhat twisted. I'd tend to disagree that it's hard, but maybe I'm looking at it from a different point of view: From a quick glance, I see the following quite distinct refactorings: - replace NDEBUG/cerr with dbg macros - introduce typedefs for existing template types - extract method in config parsing - extract resolve_conflicts() - make make_keep_list a template - extract pkginfo_t out of pkgutil class - change indentation in void pkgadd::run() which looks like it could be done before any other changes (didn't investigate that closer, though). In addition, there's the bugfix itself. That would be eight patches which all introduce one change (be it logical or cosmetical), and none of them would be 200+ lines. > If it's really > important you to read changes splitted, I will try. It seems quite important, for two reasons: - isolated changes are easier to understand, and therefore potential problems/errors are easier to spot - we can reject one particular change without rejecting the whole feature [...] > Yes, I use and like typedefs. So do Per as far as I can see, because he > introduced packages_t in pkgutil.h, but for unknown reason he do not use > it in many places of code. I suppose he just don't find enough time to > switch all set<> to proper types. This what I've done. Or one could say that he decided to use the proper types throughout the code, and forgot to switch the package_t typedef. Only he can tell. > For the most of time stl algorithms do not care which type you use, be it > vector, deque or set. It's all containers, which is nameless, and it's up > to programmer to name it. I can only agree to some extend here. While the algorithm might work either way, it can still have a performance impact. So if efficiency is a criteria, the type matters. Also, depending on the problem it matters a lot whether a container allows duplicate values etc. In those cases, it's important to know which type we're dealing with, and my argument is that it's easier to keep track of type if they're not hidden by typedefs. > The code will rather ugly and obscure without typedefs. That is why typedef > invented, btw. ;-) Well, if I see std::pair<string, string> file; I can immediately tell the type of 'file'. If it's file_t file I have to look it up. So if you ask me, it's the typedefs which are obscure, since they hide information. Don't get me wrong, typedefs are handy, but in cases like - void db_rm_pkg(const string& name, const set<string>& keep_list); + void db_rm_pkg(const string& name, const filenames_t& keep_list); I strongly prefer the explicit form. Also, for the record, I don't consider the template notation ugly, although I'm aware that not everyone shares this opinion :-). Regards, Johannes -- Johannes Winkelmann mailto:jw@smts.ch Zurich, Switzerland http://jw.smts.ch
![](https://secure.gravatar.com/avatar/6d9cfa47b02fe605d52f16450c1984ee.jpg?s=120&d=mm&r=g)
On Tue, Jul 11, 2006 at 07:43:09PM +0200, Johannes Winkelmann wrote: > - a port changing one of its binaries to be suid root (or not suid root > anymore), or any other mode change > - a port (e.g. a daemon) changing to be run as nobody from root, thus > requiring its data file to be owned by nobody:nobody > - same problem: a port starting to use a particular group (think: video, > audio etc.) which previously used 'root' User warned, if it's ok to user, then he/she pass -f option. But if mode changes happened silently, then something may break (own scripts, cron jobs...) > - human errors where a directory wasn't writable or a file not > executable, and fixed by the maintainer by changing a mode/ownership > of some files or directories User warned, maintainer noticed, maintainer got heads-up quickly after erroneous commit. Further more, if maintainer tried to pkgadd own port, he will noticed himself first. > > > > This patch fixes bugs mentioned in subject. > > > > > > > > The patch is about the same size as summary sizes of all *.cc files. > > > > But! Please, don't be scared, there are really small amount of logic > > > > changes. > > > Then it should be a small patch :-) > > > > Mmm.. no. :-) That is a point of refactoring: [...] > Well, I meant the bugfix which could be a small(er) patch when separated > from the refactorings. The bugfix is impossible with old meanings of "file". Refactoring is about to teach pkgutils to see not only filename, but pair of filename and its info. If you think that w/o splitting and debugging changes the patch would be far smaller, you are very wrong. > > > > > Most line-consuming changes are: new types, old types typedef'ed, > > > > long functions splitting, new debugging code, pkgadd.cc changes regarding > > > > indentation and long lines. > > > I'd appreciate if you could split it up. > > > > It's hard to do, because things is somewhat twisted. > I'd tend to disagree that it's hard, but maybe I'm looking at it from a > different point of view: From a quick glance, I see the following quite > distinct refactorings: > > - replace NDEBUG/cerr with dbg macros > - introduce typedefs for existing template types > - extract method in config parsing > - extract resolve_conflicts() > - make make_keep_list a template > - extract pkginfo_t out of pkgutil class > - change indentation in void pkgadd::run() which looks like it could be > done before any other changes (didn't investigate that closer, > though). I can't produce split like that. Things twisted on each other. Each such split will not compilable if others not applied. See below. > In addition, there's the bugfix itself. That would be eight patches > which all introduce one change (be it logical or cosmetical), and none > of them would be 200+ lines. > > > > If it's really > > important you to read changes splitted, I will try. > It seems quite important, for two reasons: > - isolated changes are easier to understand, and therefore potential > problems/errors are easier to spot ack > - we can reject one particular change without rejecting the whole > feature Doubtfully. There are two changes you can drop: debugging and read_config splitting. Both drops will harm readability. All others changes are necessary. I can split patch for only three pieces: debug, bugfixing by refactoring, splitting read_config. But I don't see much sense of such split. > > Yes, I use and like typedefs. So do Per as far as I can see, because he > > introduced packages_t in pkgutil.h, but for unknown reason he do not use > > it in many places of code. I suppose he just don't find enough time to > > switch all set<> to proper types. This what I've done. > Or one could say that he decided to use the proper types throughout the > code, and forgot to switch the package_t typedef. Only he can tell. > > > > For the most of time stl algorithms do not care which type you use, be it > > vector, deque or set. It's all containers, which is nameless, and it's up > > to programmer to name it. > I can only agree to some extend here. While the algorithm might work > either way, it can still have a performance impact. So if efficiency is > a criteria, the type matters. Also, depending on the problem it matters > a lot whether a container allows duplicate values etc. > > In those cases, it's important to know which type we're dealing with, > and my argument is that it's easier to keep track of type if they're not > hidden by typedefs. > > > The code will rather ugly and obscure without typedefs. That is why typedef > > invented, btw. ;-) > Well, if I see > std::pair<string, string> file; You got error here: it's pair<string,fileinfo_t>. With typedef you are safe from errors like that. > I can immediately tell the type of 'file'. If it's > file_t file > I have to look it up. You always have to look up or remember things. You have to lookup map<> functions, vector<> functions its types, differences and whatnot. Also you have to look up pkgutil functions, its arguments, the order of arguments, return types. Now you have to remember or look up few other things. I don't see the point of using explicit types. > So if you ask me, it's the typedefs which are obscure, since they hide > information. Don't get me wrong, typedefs are handy, but in cases like > - void db_rm_pkg(const string& name, const set<string>& keep_list); > + void db_rm_pkg(const string& name, const filenames_t& keep_list); > > I strongly prefer the explicit form. Also, for the record, I don't > consider the template notation ugly, although I'm aware that not > everyone shares this opinion :-). > > Regards, Johannes > -- -- Anton (irc: bd2)
![](https://secure.gravatar.com/avatar/4e374bb9f03cbbca5d9541a8bf8ec8bf.jpg?s=120&d=mm&r=g)
Hi, On Tue, Jul 11, 2006 at 23:05:39 +0400, Anton wrote: [...]
If you think that w/o splitting and debugging changes the patch would be far smaller, you are very wrong. I don't really have an interest in discussing this to death. We just expect large patches to be split into smaller patches, each introducing a single change (pretty much like the linux kernel development works).
If you submit a reasonably split set of patches we'll definitely take the time to review and discuss it, or point out when a patch violates the "one patch, one feature" rule. [typedefs]
Well, if I see std::pair<string, string> file;
You got error here: it's pair<string,fileinfo_t>. With typedef you are safe from errors like that.
In fact I merely wrote that as an example, however you instantly spotted a potential error since an explicit type was used instead of a annonymous typedef. Or to paraphrase you, "with typedef such errors are hidden from you". So bottom line is there's no "better" here, just preference, and you may want to take the maintainers' preference into consideration. HTH, Johannes -- Johannes Winkelmann mailto:jw@smts.ch Zurich, Switzerland http://jw.smts.ch
![](https://secure.gravatar.com/avatar/6d9cfa47b02fe605d52f16450c1984ee.jpg?s=120&d=mm&r=g)
On Tue, Jul 11, 2006 at 10:54:01PM +0200, Johannes Winkelmann wrote:
Hi,
On Tue, Jul 11, 2006 at 23:05:39 +0400, Anton wrote: [...]
If you think that w/o splitting and debugging changes the patch would be far smaller, you are very wrong. I don't really have an interest in discussing this to death. We just expect large patches to be split into smaller patches, each introducing a single change (pretty much like the linux kernel development works).
If you submit a reasonably split set of patches we'll definitely take the time to review and discuss it, or point out when a patch violates the "one patch, one feature" rule.
Attaching splitted patchset. Necessary for buxfixing: pkgutils-1-debug.patch pkgutils-2-bugfix.patch Not necessary: pkgutils-3-split.patch pkgutils-4-cosmetic.patch pkgutils-5-changelog.patch If you'll decide to reject debug patch, then patch 2 must be rewritten. Typedefs are used. Unfortunatelly I don't have much intentions to got headaches cause of using unnamed types. -- Anton (irc: bd2)
![](https://secure.gravatar.com/avatar/6d9cfa47b02fe605d52f16450c1984ee.jpg?s=120&d=mm&r=g)
On Wed, Jul 12, 2006 at 07:33:51PM +0400, Anton wrote:
pkgutils-3-split.patch pkgutils-4-cosmetic.patch
There is an inaccuracy between 3-split and 4-cosmetic. Line "- vector<rule_t> read_config() const;" should be done in 3-split, not 4-cosmetic. Though, it's not break compilation, thus not caught by me in first place. These try2 patches attached, others seems okay.
![](https://secure.gravatar.com/avatar/10a3ceea7f0886152bd6ee6fb28b3579.jpg?s=120&d=mm&r=g)
On Tue, 2006-07-11 at 10:41 +0200, Johannes Winkelmann wrote:
Have you though about other cases which could be potentially annoying to people, e.g. when they manually adjust permissions?
Currently they're just overwritten, which is far more annoying, as you only notice when things already broke :(
![](https://secure.gravatar.com/avatar/6d9cfa47b02fe605d52f16450c1984ee.jpg?s=120&d=mm&r=g)
Hi all, This patch adds -i (--ignore-perms) option to pkgadd, which makes possible to not bother user with permissions conflicts. To be really useful, prt-get must be modified. Say, /etc/prt-get.conf will look like: trusted-prtdir /usr/ports/core trusted-prtdir /usr/ports/opt prtdir /usr/ports/contrib Which means, that prt-get will pass -i option to all packages built from core and opt. And will not pass -i to packages from contrib. Cons: - No annoying conflicts from trusted packages; - User can choose to which repos he trust. Pros: - prt-get must be modified, which is not trivial as I can see. If you have better approach, please speak up. -- Anton (irc: bd2) p.s. Attaching two versions of the patch, first one which can be applied just after 2-bugfix patch. And second version which can be applied after 3-split and 4-cosmetic patches. Second version also fixes uninitialized variable o_force found in 3-split patch.
![](https://secure.gravatar.com/avatar/10a3ceea7f0886152bd6ee6fb28b3579.jpg?s=120&d=mm&r=g)
On Sat, 2006-07-15 at 02:17 +0400, Anton wrote:
Hi all,
This patch adds -i (--ignore-perms) option to pkgadd, which makes possible to not bother user with permissions conflicts.
To be really useful, prt-get must be modified. Say, /etc/prt-get.conf will look like: trusted-prtdir /usr/ports/core trusted-prtdir /usr/ports/opt prtdir /usr/ports/contrib
Which means, that prt-get will pass -i option to all packages built from core and opt. And will not pass -i to packages from contrib.
Cons: - No annoying conflicts from trusted packages; - User can choose to which repos he trust.
Pros: - prt-get must be modified, which is not trivial as I can see.
If you have better approach, please speak up.
Personally I'm more afraid of directories having unsafe (or too strict) permissions by accident. The solutions I see is to either detect a conflict or reject the directory and update rejmerge to handle it. Johannes and I already discussed the usefulness of something like the "trusted" feature, only with post-install scripts in mind :)
![](https://secure.gravatar.com/avatar/6d9cfa47b02fe605d52f16450c1984ee.jpg?s=120&d=mm&r=g)
On Sun, Jul 16, 2006 at 07:10:02PM +0200, Mark Rosenstand wrote:
On Sat, 2006-07-15 at 02:17 +0400, Anton wrote:
This patch adds -i (--ignore-perms) option to pkgadd, which makes possible to not bother user with permissions conflicts.
To be really useful, prt-get must be modified. Say, /etc/prt-get.conf will look like: trusted-prtdir /usr/ports/core trusted-prtdir /usr/ports/opt prtdir /usr/ports/contrib
Which means, that prt-get will pass -i option to all packages built from core and opt. And will not pass -i to packages from contrib.
Personally I'm more afraid of directories having unsafe (or too strict) permissions by accident. The solutions I see is to either detect a conflict or reject the directory and update rejmerge to handle it.
It's impossible for now, as we have one reject directory for all packages. This would possible if we introduce /var/lib/pkg/rejected/packageA/, /var/lib/pkg/rejected/packageB/, ... But I don't really like that idea.
Johannes and I already discussed the usefulness of something like the "trusted" feature, only with post-install scripts in mind :)
Then I'd like we can choose to what we trust: post-install and/or permissions changes. Maybe some kind of flags: prtdir +! /usr/ports/core prtdir ! /usr/ports/opt prtdir /usr/ports/contrib + is "I trust you to run post-install" ! is "I trust you to pass -i option" -- Anton (irc: bd2)
![](https://secure.gravatar.com/avatar/10a3ceea7f0886152bd6ee6fb28b3579.jpg?s=120&d=mm&r=g)
On Sun, 2006-07-16 at 18:13 +0400, Anton wrote:
On Sun, Jul 16, 2006 at 07:10:02PM +0200, Mark Rosenstand wrote:
On Sat, 2006-07-15 at 02:17 +0400, Anton wrote:
This patch adds -i (--ignore-perms) option to pkgadd, which makes possible to not bother user with permissions conflicts.
To be really useful, prt-get must be modified. Say, /etc/prt-get.conf will look like: trusted-prtdir /usr/ports/core trusted-prtdir /usr/ports/opt prtdir /usr/ports/contrib
Which means, that prt-get will pass -i option to all packages built from core and opt. And will not pass -i to packages from contrib.
Personally I'm more afraid of directories having unsafe (or too strict) permissions by accident. The solutions I see is to either detect a conflict or reject the directory and update rejmerge to handle it.
It's impossible for now, as we have one reject directory for all packages. This would possible if we introduce /var/lib/pkg/rejected/packageA/, /var/lib/pkg/rejected/packageB/, ...
But I don't really like that idea.
I suspect that this is an issue in theory only. pkgadd already warns when rejecting files, meaning the user should run rejmerge ASAP. It's possible (but unlikely) that the same directory will get rejected from different packages, but the overwrite in /var/lib/pkg/rejected don't really cause any trouble. (Hey, this is how it works now, except the stuff is merged directly to our filesystems without any warnings!)
Johannes and I already discussed the usefulness of something like the "trusted" feature, only with post-install scripts in mind :)
Then I'd like we can choose to what we trust: post-install and/or permissions changes. Maybe some kind of flags:
prtdir +! /usr/ports/core prtdir ! /usr/ports/opt prtdir /usr/ports/contrib
+ is "I trust you to run post-install" ! is "I trust you to pass -i option"
This seems overly complicated and cryptic to me. I think we'd be better off changing the config file format completely, like: [core] directory /usr/ports/core run-scripts yes [contrib] directory /usr/ports/contrib run-scripts no Since it scales so much better (when we want feature Z, we won't have to add yet another weird, non-logical character.)
![](https://secure.gravatar.com/avatar/6d9cfa47b02fe605d52f16450c1984ee.jpg?s=120&d=mm&r=g)
On Sun, Jul 16, 2006 at 10:07:52PM +0200, Mark Rosenstand wrote:
It's impossible for now, as we have one reject directory for all packages. This would possible if we introduce /var/lib/pkg/rejected/packageA/, /var/lib/pkg/rejected/packageB/, ...
But I don't really like that idea.
I suspect that this is an issue in theory only. pkgadd already warns when rejecting files, meaning the user should run rejmerge ASAP. It's possible (but unlikely) that the same directory will get rejected from different packages, but the overwrite in /var/lib/pkg/rejected don't really cause any trouble. (Hey, this is how it works now, except the stuff is merged directly to our filesystems without any warnings!)
Ok. Another issue with rejecting: accidentally package have /usr/share/ owned by someone else, but not root. /usr, /usr/bin, /usr/etc are fine. Half of the package installed and half is rejected. What user suppose to do?
prtdir +! /usr/ports/core prtdir ! /usr/ports/opt prtdir /usr/ports/contrib
+ is "I trust you to run post-install" ! is "I trust you to pass -i option"
This seems overly complicated and cryptic to me. I think we'd be better off changing the config file format completely, like:
[core] directory /usr/ports/core run-scripts yes
[contrib] directory /usr/ports/contrib run-scripts no
Since it scales so much better (when we want feature Z, we won't have to add yet another weird, non-logical character.)
Yep, looks much friendly. -- Anton (irc: bd2)
![](https://secure.gravatar.com/avatar/4e374bb9f03cbbca5d9541a8bf8ec8bf.jpg?s=120&d=mm&r=g)
On Sun, Jul 16, 2006 at 22:07:52 +0200, Mark Rosenstand wrote:
On Sun, Jul 16, 2006 at 07:10:02PM +0200, Mark Rosenstand wrote: [...]
Johannes and I already discussed the usefulness of something like the "trusted" feature, only with post-install scripts in mind :) [...] I think we'd be better off changing the config file format completely, like:
[core] directory /usr/ports/core run-scripts yes
[contrib] directory /usr/ports/contrib run-scripts no
Since it scales so much better (when we want feature Z, we won't have to add yet another weird, non-logical character.)
Note that while the syntax definitely looks sane, it introduces a tight coupling between the location in the ports tree and the package installation, and would therefore require us to include the repository information as meta data. Furthermore, when thinking of binary package management, this is easily spoofed (i.e. pretending to be a core package); to establish real trust, something like gnupg signatures might be better, although it would add quite a bit of additional complexity. Just my two cents here, Johannes -- Johannes Winkelmann mailto:jw@smts.ch Zurich, Switzerland http://jw.smts.ch
![](https://secure.gravatar.com/avatar/6d9cfa47b02fe605d52f16450c1984ee.jpg?s=120&d=mm&r=g)
I'm risking to got broadsided by Johannes despite again, but... Here we have proverb: nothing venture nothing have. I'll try. On Sun, Jul 16, 2006 at 11:15:44PM +0200, Johannes Winkelmann wrote:
Note that while the syntax definitely looks sane, it introduces a tight coupling between the location in the ports tree and the package installation, and would therefore require us to include the repository information as meta data.
I don't see the reason why pkgmk should generate any repo information for the package if package building from sources. prt-get already informed about from which repo it builds the package.
Furthermore, when thinking of binary package management, this is easily spoofed (i.e. pretending to be a core package); to establish real trust, something like gnupg signatures might be better, although it would add quite a bit of additional complexity.
For the binary package management, the same story: there are binary package repositories, user can choose to which repository he trusts. If user downloaded single package, then he will not use prt-get, but pkgadd. Pkgadd already have most strict policy: permissions conflicts aborts pkgadd, as for the pre/post-install scripts... pkgadd have no clue about them.
Just my two cents here, Johannes
Same here. -- Anton (irc: bd2)
participants (3)
-
Anton
-
Johannes Winkelmann
-
Mark Rosenstand