Opened 19 years ago

Closed 15 years ago

Last modified 14 years ago

#5063 closed enhancement (fixed)

RFE: sped up dependency searching and added dependent finding

Reported by: erickt@… Owned by: macports-tickets@…
Priority: Normal Milestone:
Component: base Version:
Keywords: Cc: olegb@…, jmpp@…, erickt@…
Port:

Description (last modified by jmroot (Joshua Root))

Hello All,

I whipped up a little patch to darwinports to figure out what packages depend on a particular port (opposite to the deps command). This allows some nice things. In addition, I've added a mode -R to the upgrade command that if specified, should rebuild all of the ports that depend on the one we're upgrading. I could also see having something similar to be added to the uninstall, but I haven't gotten around to that yet.

Finally, in order to make this usable, I had to speed up the loading of the portindex file. Previously, each time you did a dportsearch, each source file would have to be walked. Since I have to build a list of all the dependencies, I had to walk each file for each installed port. So to optimize this, I read each file once, and throw all the results into an array. All my testing appeared that this provided the same functionality, but I'm not sure if some other command modifies these files while port is running, so it'd break that functionality.

I've attached diffs for port.tcl and darwinports.tcl. Thanks!

-e

Attachments (6)

port.tcl.diff (6.9 KB) - added by erickt@… 19 years ago.
update for port/port.tcl against the latest head release
darwinports.tcl.diff (4.5 KB) - added by erickt@… 19 years ago.
diff against latest head release of darwinports/darwinports.tcl
port.tcl.diff2 (14.9 KB) - added by erickt@… 19 years ago.
update to add dependent port checking when uninstalling
darwinports.tcl.diff2 (6.5 KB) - added by erickt@… 19 years ago.
change to make the fast path use lowercase
dependents.diff (2.2 KB) - added by olegb@… 18 years ago.
dependents target
dependents.2.diff (1.6 KB) - added by olegb@… 18 years ago.
dependents target

Download all attachments as: .zip

Change History (30)

Changed 19 years ago by erickt@…

Attachment: port.tcl.diff added

update for port/port.tcl against the latest head release

Changed 19 years ago by erickt@…

Attachment: darwinports.tcl.diff added

diff against latest head release of darwinports/darwinports.tcl

comment:1 Changed 19 years ago by erickt@…

attachments.description: update for port.tcl against the latest head releaseupdate for port/port.tcl against the latest head release

comment:2 Changed 19 years ago by erickt@…

Note these versions were made against the latest (as of today) cvs version of darwinports

comment:3 Changed 19 years ago by jberry@…

Erick,

Wow! Looks pretty cool.

My preference would be that port searching be case-insensitive. Can you make your fast-path through dportsearch be case-sensitive (perhaps by lowercasing the names as you put them into the array, and lowe-casing before the search)? This would allow many of the other uses of dportsearch to also benefit from the fast path. What do you think?

James

comment:4 Changed 19 years ago by erickt@…

Sure that's easy enough. I didn't do that because I didn't know if there was any possibility for a name collision. If there isn't, I'd be more than happy to add it.

Oh, and one more caveat about the code. The dependents code only considers the code that has been installed, not all the dependents throughout the library. The code would be pretty simple to add that as well.

Another thing is I couldn't find where the man pages were written, so I couldn't add documentation there. Anyone know where it is?

Finally, I'd like to use long options for the different dependent modes, but I don't know tcl well enough to do that. How can that be done?

comment:5 Changed 19 years ago by jberry@…

Just thinking out loud:

We might basically want to know 4 different things, right?

(1) What are the direct dependents on this port? (deps) (2) What are the complete (recursive) dependents on this port? (rdeps) (3) What ports is this port a direct dependent of? (dep) (4) What ports is this port a (recursive) dependent of? (rdep)

I could see supporting these questions through the query interface or through direct actions:

port echo deps:portfoo port echo rdeps:portfoo port echo dep:portfoo port echo rdep:portfoo

-or-

port deps portfoo (as current) port deps -R portfoo port dep portfoo port dep -R portfoo

Does that make any sense?

comment:6 Changed 19 years ago by toby@…

Component: dportsbase

Changed 19 years ago by erickt@…

Attachment: port.tcl.diff2 added

update to add dependent port checking when uninstalling

comment:7 Changed 19 years ago by erickt@…

attachments.isobsolete: 01

Changed 19 years ago by erickt@…

Attachment: darwinports.tcl.diff2 added

change to make the fast path use lowercase

comment:8 Changed 19 years ago by erickt@…

attachments.isobsolete: 01

comment:9 Changed 19 years ago by erickt@…

All I have to do left to add James's suggestion is to resolve how the dependency is called. Right now it still uses the "dependents"/"dependents -R". Also, I'd like to add something analogous to the uninstaller that deactivates dependent ports on the port we're deactivating. But that's for another time...

comment:10 Changed 19 years ago by jberry@…

Hi Erick,

This is getting big. Anything you can do to limit the scope of what's going into this patch would probably be good.

A couple of questions without too much investigation on my part:

(1) It looks like your dependent fetching only goes one level deep? Doesn't it need to be recursive or

iterative (add found dependents to the list of ports still to check)? Or did I miss that?

(2) You changed foreachport to accomodate ill-formed portlists. I'd rather see the portlist kept

maintained in a canonicalized form rather than having to worry what's it's form is whereever it's used. So you could either always make sure to build it in the canonical form, or provide a means of canonicalizing it before it's used as a "portlist". Did that make any sense? There's routine add_to_portlist, or something, that will add a port to the portlist, making sure to canonicalize it in the process.

comment:11 Changed 18 years ago by erickt@…

Hi James,

I thought I responded but I guess I didn't :)

(In reply to comment #10)

This is getting big. Anything you can do to limit the scope of what's going into this patch would probably be good.

How would you like me to break it up? I could separate the "uninstall" section into it's own bug and make it depend on this one, but that'd just be a little complicated to keep track of things on my side. I could do that though.

(1) It looks like your dependent fetching only goes one level deep? Doesn't it need to be recursive or

iterative (add found dependents to the list of ports still to check)? Or did I miss that?

There is the ability to find all of the dependent tasks. Because I didnt' want to modify the portindex file, I just determine all the dependents at the same time, in the "get_all_dependents" function. In my testing this is quite fast (most of the time is actually spent opening and reading the portindex file). You're right though, there is a bug in "get_dependents", it only gets one level deeper if you specify "find_all". I'll fix that.

(2) You changed foreachport to accomodate ill-formed portlists. I'd rather see the portlist kept

maintained in a canonicalized form rather than having to worry what's it's form is whereever it's used. So you could either always make sure to build it in the canonical form, or provide a means of canonicalizing it before it's used as a "portlist". Did that make any sense? There's routine add_to_portlist, or something, that will add a port to the portlist, making sure to canonicalize it in

the

process.

Are you referring to my added function "foreachportname"? That function simply will convert a list of strings into the canonical portfile spec form. If I modified the "foreachport", then thats a bug. Or are you referring to the code I factored out in "portname_to_portspec"? That was just removing common code throughout that script to make things more organized. I did need to add a hack to "foreachportname" though as I some values weren't properly defined in the return value of "dportsearch". I'm not sure if that was a in-development bug, so it can be removed when "dportsearch" is ready. I did mark that code as XXX, if it helps.

-e

comment:12 Changed 18 years ago by jberry@…

Erick,

Thanks for your comments. We'll consider some of this code for inclusion into 1.3. We've frozen the feature set for 1.2. -jdb

comment:13 Changed 18 years ago by jmpp@…

blocked: 3951

comment:14 Changed 18 years ago by jmpp@…

Cc: olegb@… added

Hey Ole, I'm Cc'ing you here cause this thread is on the same line as your "rebuild dependents (-R)" work. You mind bringing your expertise in? We're considering this for inclusion in 1.3, plans are young and open.

I also made bug #3951 dependent on this one, a specific example of where this feature might come in handy.

-jmpp

comment:15 Changed 18 years ago by jmpp@…

Cc: jmpp@… added

comment:16 Changed 18 years ago by olegb@…

why is it that this recursive stuff is all ways an reimplementation of the registry's list_dependents?

I dont think we need more cruft in darwinports.tcl - not stuff that is implemented else where.

Changed 18 years ago by olegb@…

Attachment: dependents.diff added

dependents target

Changed 18 years ago by olegb@…

Attachment: dependents.2.diff added

dependents target

comment:17 Changed 18 years ago by olegb@…

attachments.isobsolete: 01

comment:19 Changed 18 years ago by jmpp@…

Hello Erick! Ole Jensen was working on a similar note as you, a dependents checking & rebuild target & cli flag, respectively, but you also add other functionality in your patches here. His work was committed to CVS HEAD some days ago, so could I ask you to please refresh your checkout and make a patch *only* for the "other stuff"? We're still interested in your contributions!

But I say "other stuff" because I'm not too clear on what you want to accomplish, because your patch is not in unidiff format. When you diff the relevant files to create the patch, please supply the -u flag to diff(1), it makes the results much easier for everybody to parse and understand.

Lastly, when you upload the new patch, please remember to mark as "obsolete"' the now old attachments (I think every single one up there as of now), as it makes the bug history clearer.

Thanks for your contributions and interest in DarwinPorts!

-jmpp

comment:20 Changed 18 years ago by olegb@…

Erick, possible to get a diff -u against HEAD of your "deps" changes?

comment:21 Changed 18 years ago by markd@…

Type: defectenhancement

comment:22 Changed 17 years ago by nox@…

Cc: olegb@… jmpp@… added; olegb@… jmpp@… removed
Milestone: MacPorts base enhancements
Priority: ExpectedNormal
Summary: sped up dependency searching and added dependent findingRFE: sped up dependency searching and added dependent finding
Version: 1.0

comment:23 Changed 15 years ago by jmroot (Joshua Root)

Cc: erickt@… added
Description: modified (diff)
Resolution: fixed
Status: newclosed

It looks like everything applicable here has been done.

comment:24 Changed 14 years ago by jmroot (Joshua Root)

Milestone: MacPorts Future
Note: See TracTickets for help on using tickets.