Re: [PATCH] Skip invalid compiled file found, continue searching path.

[David Kastrup]

Jan Nieuwenhuizen  writes:

> Hi,
>
> As per chat with Ludovic on #guile (thanks!) find attached another
> approach to allow switching incrementally from guile-2.0 to guile-2.2:
> any invalid compiled files are skipped and we continue searching the
> GUILE_LOAD_COMPILED_PATH instead of throwing early.

Shouldn't we worry about switching _to_ guile-2.0 first?

-- 
David Kastrup

[PATCH] Skip invalid compiled file found, continue searching path.

[Jan Nieuwenhuizen]

Hi,

As per chat with Ludovic on #guile (thanks!) find attached another
approach to allow switching incrementally from guile-2.0 to guile-2.2:
any invalid compiled files are skipped and we continue searching the
GUILE_LOAD_COMPILED_PATH instead of throwing early.

Greetings,
Jan

>From f4f53b48c1d5ff42ecc66279c3b1cfcfb09d6757 Mon Sep 17 00:00:00 2001
From: Jan Nieuwenhuizen 
Date: Fri, 11 Mar 2016 14:58:09 +0100
Subject: [PATCH] Skip invalid compiled file found, continue searching path.

* libguile/vm.c (load_compiled_with_vm_catch_handler,
  do_try_scm_call_0): New static function.
  (scm_load_compiled_with_vm): Use them to implement not throwing
  if new argument EXCEPTION_ON_NOT_FOUND_P is not SCM_BOOL_TRUE.
* libguile/load.c (search_path): Take optional output argument
  PATH_REMAINING.
  (scm_primitive_load_path): Use it.  Take optional argument
  LOAD_COMPILED_PATH.  Skip any invalid compiled file found and
  continue searching scm_loc_load_compiled_path.
---
 libguile/load.c | 55 ---
 libguile/vm.c   | 27 ---
 libguile/vm.h   |  2 +-
 3 files changed, 65 insertions(+), 19 deletions(-)

diff --git a/libguile/load.c b/libguile/load.c
index d26f9fc..a6b87cf 100644
--- a/libguile/load.c
+++ b/libguile/load.c
@@ -582,12 +582,16 @@ compiled_is_fresh (SCM full_filename, SCM compiled_filename,
file name that we find in the path.  Otherwise only return a file if
it is newer than SOURCE_STAT_BUF, otherwise issuing a warning if we
see a stale file earlier in the path, setting *FOUND_STALE_FILE to 1.
+
+   If PATH_REMAINING is not NULL, it is set to the part of PATH that was
+   not yet searched.
   */
 static SCM
 search_path (SCM path, SCM filename, SCM extensions, SCM require_exts,
  struct stat *stat_buf,
  SCM source_file_name, struct stat *source_stat_buf,
- int *found_stale_file)
+ int *found_stale_file,
+ SCM *path_remaining)
 {
   struct stringbuf buf;
   char *filename_chars;
@@ -724,6 +728,8 @@ search_path (SCM path, SCM filename, SCM extensions, SCM require_exts,
 
  end:
   scm_dynwind_end ();
+  if (path_remaining)
+path_remaining = 
   return result;
 }
 
@@ -781,7 +787,7 @@ SCM_DEFINE (scm_search_path, "search-path", 2, 0, 1,
 require_exts = SCM_BOOL_F;
 
   return search_path (path, filename, extensions, require_exts, _buf,
-  SCM_BOOL_F, NULL, NULL);
+  SCM_BOOL_F, NULL, NULL, NULL);
 }
 #undef FUNC_NAME
 
@@ -806,7 +812,7 @@ SCM_DEFINE (scm_sys_search_load_path, "%search-load-path", 1, 0, 0,
   SCM_VALIDATE_STRING (1, filename);
 
   return search_path (*scm_loc_load_path, filename, *scm_loc_load_extensions,
-  SCM_BOOL_F, _buf, SCM_BOOL_F, NULL, NULL);
+  SCM_BOOL_F, _buf, SCM_BOOL_F, NULL, NULL, NULL);
 }
 #undef FUNC_NAME
 
@@ -969,14 +975,19 @@ SCM_DEFINE (scm_primitive_load_path, "primitive-load-path", 0, 0, 1,
 "depending on the optional second argument,\n"
 "@var{exception_on_not_found}.  If it is @code{#f}, @code{#f}\n"
 "will be returned.  If it is a procedure, it will be called\n"
-"with no arguments.  Otherwise an error is signalled.")
+"with no arguments.  Otherwise an error is signalled."
+"If the optional third argument,\n"
+"@var{load_compiled_path} is given, use it to search for compiled files\n"
+"instead of @var{*scm_loc_load_compiled_path}.")
 #define FUNC_NAME s_scm_primitive_load_path
 {
   SCM filename, exception_on_not_found;
   SCM full_filename, compiled_filename;
   SCM hook = *scm_loc_load_hook;
   struct stat stat_source, stat_compiled;
+  SCM load_compiled_path;
   int found_stale_compiled_file = 0;
+  SCM load_compiled_path_remaining = SCM_EOL;
 
   if (scm_is_true (hook) && scm_is_false (scm_procedure_p (hook)))
 SCM_MISC_ERROR ("value of %load-hook is neither a procedure nor #f",
@@ -988,21 +999,27 @@ SCM_DEFINE (scm_primitive_load_path, "primitive-load-path", 0, 0, 1,
 	 single argument (the file name).  */
   filename = args;
   exception_on_not_found = SCM_UNDEFINED;
+  load_compiled_path = *scm_loc_load_compiled_path;
 }
   else
 {
-  /* Starting from 1.9, this function takes 1 required and 1 optional
-	 argument.  */
+  /* Starting from 1.9, this function takes 1 required and 1
+	 optional argument.
+
+ Starting from 2.1.2, this function takes 1 required and 2
+	 optional arguments.  */
   long len;
 
   SCM_VALIDATE_LIST_COPYLEN (SCM_ARG1, args, len);
-  if (len < 1 || len > 2)
+  if (len < 1 || len > 3)
 	scm_error_num_args_subr (FUNC_NAME);
 
   filename = SCM_CAR (args);
   SCM_VALIDATE_STRING (SCM_ARG1, filename);
 
   exception_on_not_found = len > 1 ? SCM_CADR (args) : SCM_UNDEFINED;
+  load_compiled_path = len < 3 ? 

Re: Elisp branch ready for merge (??)

[Mathieu Lirzin]

Christopher Allan Webber  writes:

> Mathieu Lirzin writes:
>
[...]
>> Small nitpick.  Could you remove the extra spaces at the start of
>> indented lines?  :)
>>
>> I know this practice is/was a common practice in Guile but even if it
>> looks prettier with indentation, this is not the proper change log
>> format described by GCS and recognized by 'change-log-mode' in Emacs.
>
> I could... but I'm hesitant to do so if it's not the standard ways to do
> things in Guile's repository.  I'll leave that question to whoever is
> interested in merging it.  If they'd like me to change it, I'll do it,
> and since they have commit access, I consider them the authority.

Sure, it will be an opportunity to discuss this major issue!  ;)

-- 
Mathieu Lirzin

Re: Elisp branch ready for merge (??)

[Christopher Allan Webber]

Taylan Ulrich Bayırlı/Kammer writes:

> Christopher Allan Webber  writes:
>
>> Well, I didn't think I'd have time to do this (and in a sense I didn't)
>> but:
>>   https://gitlab.com/dustyweb/guile/tree/merge-bipt-elisp-wip
>>
>> I've rebased the whole branch against git master and added ChangeLog
>> style entries.  "make check" is passing.  It seems to me that it's ready
>> for merge.  I did the best I could on the ChangeLog additions, both with
>> my limited ChangeLog experience and from my limited Guile internals
>> experience.  So, corrections welcome, but otherwise...
>
> Exciting!
>
> Small remark: the "title" line of the commit messages should be complete
> sentences.

Yes, though I didn't write them.  I also don't know in each case what a
complete sentence would be.  I did my best job by filling in the
ChangeLog style part.  Often times my figuring out the ChangeLog stuff
was based on some sentence fragment.

Changing the one part that is the original author's writing to something
different... I can do it by attempting to guess, but I'm worried about
removing that context.

One thing I could do is leave in the description: "Original title: foo"

What do you think of that?

>> I think we really should not delay on this, and we should try to merge
>> this as soon as possible.  This already has bitrotted before, and if we
>> wait on it, it could bitrot again.  It would be great to get this pulled
>> into Guile proper.
>>
>> Plus it would be a nice bullet point in the next release! :)
>
> +1
>
> Would also be good that one doesn't need to keep two versions of Guile
> around when hacking on Guile-Emacs.

I agree! :)

Re: Elisp branch ready for merge (??)

[Christopher Allan Webber]

Mathieu Lirzin writes:

> Christopher Allan Webber  writes:
>
>> Well, I didn't think I'd have time to do this (and in a sense I didn't)
>> but:
>>   https://gitlab.com/dustyweb/guile/tree/merge-bipt-elisp-wip
>>
>> I've rebased the whole branch against git master and added ChangeLog
>> style entries.  "make check" is passing.  It seems to me that it's ready
>> for merge.  I did the best I could on the ChangeLog additions, both with
>> my limited ChangeLog experience and from my limited Guile internals
>> experience.  So, corrections welcome, but otherwise...
>
> Nice!
>
> Small nitpick.  Could you remove the extra spaces at the start of
> indented lines?  :)
>
> I know this practice is/was a common practice in Guile but even if it
> looks prettier with indentation, this is not the proper change log
> format described by GCS and recognized by 'change-log-mode' in Emacs.

I could... but I'm hesitant to do so if it's not the standard ways to do
things in Guile's repository.  I'll leave that question to whoever is
interested in merging it.  If they'd like me to change it, I'll do it,
and since they have commit access, I consider them the authority.

Re: Elisp branch ready for merge (??)

[Mathieu Lirzin]

Christopher Allan Webber  writes:

> Well, I didn't think I'd have time to do this (and in a sense I didn't)
> but:
>   https://gitlab.com/dustyweb/guile/tree/merge-bipt-elisp-wip
>
> I've rebased the whole branch against git master and added ChangeLog
> style entries.  "make check" is passing.  It seems to me that it's ready
> for merge.  I did the best I could on the ChangeLog additions, both with
> my limited ChangeLog experience and from my limited Guile internals
> experience.  So, corrections welcome, but otherwise...

Nice!

Small nitpick.  Could you remove the extra spaces at the start of
indented lines?  :)

I know this practice is/was a common practice in Guile but even if it
looks prettier with indentation, this is not the proper change log
format described by GCS and recognized by 'change-log-mode' in Emacs.

-- 

Mathieu Lirzin

Re: Elisp branch ready for merge (??)

[Taylan Ulrich Bayırlı/Kammer]

Christopher Allan Webber  writes:

> Well, I didn't think I'd have time to do this (and in a sense I didn't)
> but:
>   https://gitlab.com/dustyweb/guile/tree/merge-bipt-elisp-wip
>
> I've rebased the whole branch against git master and added ChangeLog
> style entries.  "make check" is passing.  It seems to me that it's ready
> for merge.  I did the best I could on the ChangeLog additions, both with
> my limited ChangeLog experience and from my limited Guile internals
> experience.  So, corrections welcome, but otherwise...

Exciting!

Small remark: the "title" line of the commit messages should be complete
sentences.

> I think we really should not delay on this, and we should try to merge
> this as soon as possible.  This already has bitrotted before, and if we
> wait on it, it could bitrot again.  It would be great to get this pulled
> into Guile proper.
>
> Plus it would be a nice bullet point in the next release! :)

+1

Would also be good that one doesn't need to keep two versions of Guile
around when hacking on Guile-Emacs.

>  - Chris

Taylan