[Yum-devel] Patch: Hard-link packages that have already been downloaded.

[Ryan Lynch]

On Mon, Sep 21, 2009 at 12:31, James Antill <james at fedoraproject.org> wrote:
> On Sat, 2009-09-19 at 00:15 -0400, Ryan Lynch wrote:
>> On Fri, Sep 18, 2009 at 00:15, James Antill <james at fedoraproject.org> wrote:
>> > 4. I'm not sure you can just use 'path' for symlink's argument, might
>> > need to do realpath() ... or at least normalize it.
>  The big problem here is what the value of path is, which doesn't matter
> for link() but does for symlink(). For instance if you do:
>
> --package-cache /path/to/pkgs blah
>
> ...then the symlink for foo is going to be pointing
> to /path/to/pkgs/foo, which might not be available from http (where the
> reposync ends up being served).
>  I guess we can just say "don't do that", but it'd be nice to not have
> to.

I think I see what you mean--whatever type of path you pass to
'--package-cache' gets written into the symlinks.  If it's a relative
path, you get relative symlinks... if the path includes a symlink, you
get multiple symlink indirection.

I originally intended this behavior, but the use case that inspired it
has gone away.  I'll run 'os.path.realpath()' on the paths that are
passed as parameters to the '--package-cache' option.

But since 'localpkgs()' recursively searches sub-directories of the
'--package-cache' parameters, this raises another question:  Should I
also resolve symlinks in sub-directories?  I'm thinking something
like:

 - change the 'os.stat()' call back to an 'os.lstat()' so we can
distinguish what's a symlink, and
 - add an 'if' clause, preceding the existing 'if' block, that calls
'stat.S_ISLNK(st.st_mode)' to check whether we've hit a symlink, and
if so:
 - resolve the link path with 'os.path.realpath(fn)', and
 - pass the resolved path into the existing 'if' block, instead of the
symlink aliased path.

I think this should guarantee that any symlinks will be absolute and
not involve any multiple indirections.


>> > 6. Might as well add the pkgs you download to the cache.
>>
>> Again, I'm not quite sure what this means, since I'm not that familiar
>> with the YUM/RPM idioms.  Can you clarify?
>
>  Just in case there are two identical pkgs. in the repo. for the
> reposync run ... might as well get the second one for free.

I think I see what you're saying.  I'm looking at the block starting
'if opts.tempcache', and the methods 'getCacheDir()' and
'setCacheDir()'--I'll take a look at the underlying code, and I'll see
if I can figure out how to do that.


>> neither of which appealed to me as much as the current
>> 'try-except-else usage.  And as much as I hate to fall back on this
>> justification, the 'try-except-else' logic just feels more Pythonic.
>> But I understand that reasonable people might disagree, so let me know
>> your preferences and I can change it, if required.
>
>  I have a personal preference, but esp. here it doesn't usually matter.
> I don't think we have any hard rules about it anywhere new.

As long as you're OK with it, that's all that matters to me.


>> I realize that I might need to break this down into smaller patches.
>> Can you let me know whether I'm on the right track, first?
>
>  It looks fine, no need to break it into more patches.

Good, I'll start working on the points I noted, above, and I'll submit
another revision when I have the chance.

-Ryan