find-versioned-module bugs

find-versioned-module bugs

[Andy Wingo]

Hi Julian,

~/src/guile$ meta/guile
scheme@(guile-user)> (find-versioned-module "ice-9/" "q" '() %load-path )
$1 = "/opt/guile/share/guile/2.0/ice-9//q.scm"

scheme@(guile-user)> %load-path
$2 = ("/home/wingo/src/guile/guile-readline" "/home/wingo/src/guile"
"/home/wingo/src/guile/module" "/home/wingo/src/guile/module"
"/opt/guile/share/guile/2.0" "/opt/guile/share/guile/site"
"/opt/guile/share/guile")

As you see this function is preferring modules from the end of the
%load-path. In addition it has adds an unnecessary "/". Do you have time
to fix it? :)

Andy
-- 
http://wingolog.org/

Re: find-versioned-module bugs

[Julian Graham]

Hey Andy,


> As you see this function is preferring modules from the end of the
> %load-path. In addition it has adds an unnecessary "/". Do you have time
> to fix it? :)

Eep!  Sure, I'll try to take a look this evening.


Regards,
Julian

Re: find-versioned-module bugs

[Julian Graham]

Howdy,


>> As you see this function is preferring modules from the end of the
>> %load-path. In addition it has adds an unnecessary "/". Do you have time
>> to fix it? :)
>
> Eep!  Sure, I'll try to take a look this evening.

Fixed!  See commit dc232ed059a0af5955d21f077da88af6fdc562a0.


Regards,
Julian

Re: find-versioned-module bugs

[Andy Wingo]

On Thu 10 Jun 2010 06:25, Julian Graham <joolean@gmail.com> writes:

>>> As you see this function is preferring modules from the end of the
>>> %load-path. In addition it has adds an unnecessary "/". Do you have time
>>> to fix it? :)
>>
>> Eep!  Sure, I'll try to take a look this evening.
>
> Fixed!  See commit dc232ed059a0af5955d21f077da88af6fdc562a0.

Thanks! Now, a further request, which you may turn down of course :-).
This function currently looks in all dirs in the load path for all ways
that versions can be represented, hence the stat explosion. Instead of
enumerating the entire set of compatible modules, why not simply stop
with the first one? I suspect this would respect the user's expectations
just as well.

What do you think? :)

Andy
-- 
http://wingolog.org/

Re: find-versioned-module bugs

[Julian Graham]

Hey Andy,


> Thanks! Now, a further request, which you may turn down of course :-).
> This function currently looks in all dirs in the load path for all ways
> that versions can be represented, hence the stat explosion. Instead of
> enumerating the entire set of compatible modules, why not simply stop
> with the first one? I suspect this would respect the user's expectations
> just as well.
>
> What do you think? :)

There's a method to the madness (if I understand the question) -- it
stems from an earlier conversation we (well, Neil and I) had [0] about
wanting to load the most recent version of a library when multiple
matches are available for a partially-specified version reference.  So
just because we've found, say, a module with version (6 1) that
matches our version reference of (6), we might find a module with
version (6 2) further in our search.  Of course, R6RS says:

"When more than one library is identified by a library reference, the
choice of libraries is determined in some implementation-dependent
manner."

So we can certainly change this behavior.  Shall I go ahead and do that?


Regards,
Julian


[0] - http://www.mail-archive.com/guile-devel@gnu.org/msg03655.html

Re: find-versioned-module bugs

[Andy Wingo]

Hi Julian,

On Thu 10 Jun 2010 16:02, Julian Graham <joolean@gmail.com> writes:

> wanting to load the most recent version of a library when multiple
> matches are available for a partially-specified version reference.  So
> just because we've found, say, a module with version (6 1) that
> matches our version reference of (6), we might find a module with
> version (6 2) further in our search.  Of course, R6RS says:
>
> "When more than one library is identified by a library reference, the
> choice of libraries is determined in some implementation-dependent
> manner."
>
> So we can certainly change this behavior.  Shall I go ahead and do that?

Well, not to contradict Neil, but I do think changing is the right
answer. In http://www.mail-archive.com/guile-devel@gnu.org/msg03655.html
you had asked:

> > * Should we establish some rules for what you get when you don't
> > specify a version?

And Neil replied:

> Yes!  The latest available?

While this was a noble sentiment, and a consistent strategy, it's not
the right thing IMO. Versions were not a well-considered addition to the
spec, and we should not feel compelled to (a) promote them, or (b) to
implement them ideally (according to their ideals).

In our case, my opinion is that we should change the rule to be, "the
first compatible version found in the path"; though perhaps we should
wait for confirmation from Ludovic.

Cheers,

Andy
-- 
http://wingolog.org/

Re: find-versioned-module bugs

[Andy Wingo]

On Thu 10 Jun 2010 16:49, Andy Wingo <wingo@pobox.com> writes:

> In our case, my opinion is that we should change the rule to be, "the
> first compatible version found in the path"; though perhaps we should
> wait for confirmation from Ludovic.

I was just looking at the current situation, and there are a few bugs.

  1. Precompiled files are never used; only the fallback path (~/.cache)
     is used. This is because resolve-module calls `load' in the
     versioned case, and only `primitive-load-path' scours
     `%load-compiled-path'. The solution is going to be complicated, and
     require some refactoring. I don't want to implement the mtime dance
     again.

  2. Version order: Hoo, dunno. See above. Is "first compatible" the
     right thing, or should we be smarter (and more wasteful)?

Any thoughts? :)

Andy
-- 
http://wingolog.org/

Re: find-versioned-module bugs

[Julian Graham]

Hi Andy,


>  1. Precompiled files are never used; only the fallback path (~/.cache)
>     is used. This is because resolve-module calls `load' in the
>     versioned case, and only `primitive-load-path' scours
>     `%load-compiled-path'. The solution is going to be complicated, and
>     require some refactoring. I don't want to implement the mtime dance
>     again.

Ah, yes, I remember -- FWIW, `find-versioned-module' was supposed to
be a version-aware formulation of `primitive-load-path'.  Well, uh,
what should we do?  I don't suppose it'd make sense to move some of
the precompiled-file-checking code from `primitive-load-path' into a
place where it could be used by, say, `primitive-load', would it?


>  2. Version order: Hoo, dunno. See above. Is "first compatible" the
>     right thing, or should we be smarter (and more wasteful)?

I don't feel strongly either way -- "smarter's" already working (I
think), but "first compatible" is pretty easy to move to.  Let me know
how you'd like to proceed and I'll make it so.

Sorry I've been sluggish about addressing these!


Regards,
Julian

Re: find-versioned-module bugs

[Andy Wingo]

Hi Julian,

On Tue 15 Jun 2010 07:02, Julian Graham <joolean@gmail.com> writes:

>>  1. Precompiled files are never used; only the fallback path (~/.cache)
>>     is used. This is because resolve-module calls `load' in the
>>     versioned case, and only `primitive-load-path' scours
>>     `%load-compiled-path'. The solution is going to be complicated, and
>>     require some refactoring. I don't want to implement the mtime dance
>>     again.
>
> Ah, yes, I remember -- FWIW, `find-versioned-module' was supposed to
> be a version-aware formulation of `primitive-load-path'.  Well, uh,
> what should we do?  I don't suppose it'd make sense to move some of
> the precompiled-file-checking code from `primitive-load-path' into a
> place where it could be used by, say, `primitive-load', would it?

I don't know, all that code is tricky. How about having
find-versioned-module return a tail instead of a full path, then pass
that tail to primitive-load-path? It will cause some slight duplication
of effort but it will find the .go correctly.

Andy
-- 
http://wingolog.org/

Re: find-versioned-module bugs

[Ludovic Courtès]

Hello,

Andy Wingo <wingo@pobox.com> writes:

> While this was a noble sentiment, and a consistent strategy, it's not
> the right thing IMO. Versions were not a well-considered addition to the
> spec, and we should not feel compelled to (a) promote them, or (b) to
> implement them ideally (according to their ideals).

+1

In particular we could choose to ignore version specs altogether.

(Ironically, given the tendency of WG1/2 to ignore R6RS, along with
implementors stating they implement “languages descending from Scheme”
as opposed to just “Scheme” [0], it may well be that ‘6’ will be the
only version number ever used.)

> In our case, my opinion is that we should change the rule to be, "the
> first compatible version found in the path"; though perhaps we should
> wait for confirmation from Ludovic.

I’m a bit lost.  As you said on IRC, we should probably start by looking
at what other implementations do (for those implementations that resolve
modules at run-time, that is.)

Thanks,
Ludo’.

[0] http://www.racket-lang.org/new-name.html

Re: find-versioned-module bugs

[Andy Wingo]

On Wed 16 Jun 2010 01:12, ludo@gnu.org (Ludovic Courtès) writes:

> Andy Wingo <wingo@pobox.com> writes:
>
>> In our case, my opinion is that we should change the rule to be, "the
>> first compatible version found in the path"; though perhaps we should
>> wait for confirmation from Ludovic.
>
> I’m a bit lost.  As you said on IRC, we should probably start by looking
> at what other implementations do (for those implementations that resolve
> modules at run-time, that is.)

Well I am a little lost too. Currently I would be fine with not encoding
version information into the path at all; the only way to support
multiple installed versions would be %load-path trickery. So instead of
rnrs/6/base.scm, just rnrs/base.scm (though the module still stores its
version).

Andy
-- 
http://wingolog.org/

Re: find-versioned-module bugs

[Julian Graham]

[-- Attachment #1: Type: text/plain, Size: 481 bytes --]

Hey Andy,


> I don't know, all that code is tricky. How about having
> find-versioned-module return a tail instead of a full path, then pass
> that tail to primitive-load-path? It will cause some slight duplication
> of effort but it will find the .go correctly.

Sure!  Find attached an implementation of `find-versioned-module' that
does precisely that -- although it looks like we've gone in a slightly
different direction since this email was written... ;-)


Regards,
Julian

[-- Attachment #2: find-versioned-module.scm --]
[-- Type: text/x-scheme, Size: 3586 bytes --]

(define (find-versioned-module dir-hint name version-ref roots)
  (define (subdir-pair-less pair1 pair2)
    (define (numlist-less lst1 lst2)
      (or (null? lst2) 
          (and (not (null? lst1))
               (cond ((> (car lst1) (car lst2)) #t)
                     ((< (car lst1) (car lst2)) #f)
                     (else (numlist-less (cdr lst1) (cdr lst2)))))))
    (not (numlist-less (car pair2) (car pair1))))
  (define (match-version-and-file pair)
    (and (version-matches? version-ref (car pair))
         (let ((filenames                            
                (filter (lambda (file-pair)
                          (let* ((file (in-vicinity (car file-pair) 
                                                    (cdr file-pair)))
                                 (s (false-if-exception (stat file))))
                            (and s (eq? (stat:type s) 'regular))))
                        (map (lambda (ext)
                               (cons (cadr pair)
                                     (in-vicinity (cddr pair) 
                                                  (string-append name ext))))
                             %load-extensions))))
           (and (not (null? filenames))
                (cons (car pair) (car filenames))))))
    
  (define (match-version-recursive root-pairs leaf-pairs)
    (define (filter-subdirs root-pairs ret)
      (define (filter-subdir root-pair dstrm subdir-pairs)
        (let ((entry (readdir dstrm)))
          (if (eof-object? entry)
              subdir-pairs
              (let* ((subdir (in-vicinity (cddr root-pair) entry))
                     (dir (in-vicinity (cadr root-pair) subdir))
                     (num (string->number entry))
                     (num (and num (exact? num) 
                               (append (car root-pair) (list num)))))
                (if (and num (eq? (stat:type (stat dir)) 'directory))
                    (filter-subdir root-pair dstrm 
                                   (cons (cons num (cons (cadr root-pair) 
                                                         subdir))
                                         subdir-pairs))
                    (filter-subdir root-pair dstrm subdir-pairs))))))
      
      (or (and (null? root-pairs) ret)
          (let* ((rp (car root-pairs))
                 (dir (in-vicinity (cadr rp) (cddr rp)))
                 (dstrm (false-if-exception (opendir dir))))
            (if dstrm
                (let ((subdir-pairs (filter-subdir rp dstrm '())))
                  (closedir dstrm)
                  (filter-subdirs (cdr root-pairs) 
                                  (or (and (null? subdir-pairs) ret)
                                      (append ret subdir-pairs))))
                (filter-subdirs (cdr root-pairs) ret)))))
    
    (or (and (null? root-pairs) leaf-pairs)
        (let ((matching-subdir-pairs (filter-subdirs root-pairs '())))
          (match-version-recursive
           matching-subdir-pairs
           (append leaf-pairs (filter pair? (map match-version-and-file 
                                                 matching-subdir-pairs)))))))

  (define (make-root-pair root) (cons '() (cons root dir-hint)))

  (let* ((root-pairs (map make-root-pair roots))
         (matches (if (null? version-ref) 
                      (filter pair? (map match-version-and-file root-pairs))
                      '()))
         (matches (append matches (match-version-recursive root-pairs '()))))
    (and (null? matches) (error "No matching modules found."))
    (cddar (sort matches subdir-pair-less))))

Re: find-versioned-module bugs

[Andy Wingo]

On Thu 17 Jun 2010 14:58, Julian Graham <joolean@gmail.com> writes:

>> I don't know, all that code is tricky. How about having
>> find-versioned-module return a tail instead of a full path, then pass
>> that tail to primitive-load-path? It will cause some slight duplication
>> of effort but it will find the .go correctly.
>
> Sure!  Find attached an implementation of `find-versioned-module' that
> does precisely that -- although it looks like we've gone in a slightly
> different direction since this email was written... ;-)

We did get to discuss a bit on IRC, but for the list: I have removed
support for searching for versioned modules in the filesystem. We could
add it back later, but I would like for it to be in a more
user-extensible form if we do that.

I should also apologize in public for leading you down this goose chase
:/ Oh well!

Regards,

Andy
-- 
http://wingolog.org/