unofficial mirror of bug-guile@gnu.org 
 help / color / mirror / Atom feed
From: Maxim Cournoyer <maxim.cournoyer@gmail.com>
To: Amirouche <amirouche@hyper.dev>
Cc: "66046@debbugs.gnu.org" <66046@debbugs.gnu.org>
Subject: bug#66046: Relative includes in R7RS define-library seem broken
Date: Tue, 14 Nov 2023 08:57:48 -0500	[thread overview]
Message-ID: <87il64cu8z.fsf@gmail.com> (raw)
In-Reply-To: <3jwzxtCW91bvW_AqM4x1Xpm-kdOmiBYvVFfKneO0-Ls556aEA5wWugMgOShhtaP0cpfvxta2wcuqZX3jSelpVE4QpmTo2zjX92QQ5owzp04=@hyper.dev> (amirouche@hyper.dev's message of "Sat, 11 Nov 2023 11:58:49 +0000")

Hello,

Amirouche <amirouche@hyper.dev> writes:

> If I am not mistaken, the patch is not backward compatible.
>
> The problem with the current patch is that it force the included file 
> to be next to the including file, there is no fallback  mechanism. 
> The algorithm should be dynamic using an ordered list a priority to
> the favorite behavior. 
>
> The most relevant hunk is:
>
> --- a/module/ice-9/psyntax.scm
> +++ b/module/ice-9/psyntax.scm
> @@ -3260,15 +3260,20 @@
>    (let ((syntax-dirname (lambda (stx)
>                            (define src (syntax-source stx))
>                            (define filename (and src (assq-ref src 'filename)))
> -                          (and (string? filename)
> -                               (dirname filename)))))
> +                          (define source-file-name
> +                            (fluid-ref compilation-source-file-name))
> +                          (or (and source-file-name
> +                                   (dirname source-file-name))
> +                              (and (string? filename)
> +                                   (dirname filename))))))
>
> Here the code says: the included file must be in (syntax-dirname). 
> It is preferable to have fallbacks, to be backward compatible.

It also falls back to the existing behavior, which is of picking up the
parent directory of the port's file name (that is, the parent directory
of the source file using the 'include' syntax), per the 'or' above.
Isn't that sufficient?

> `syntax-dirname' must be `syntax-dirnames' to return candidate directories 
> sorted list with biggest priority coming first where to find included 
> files.

I'm not sure what algorithm you are suggesting here; but it seems it'd
be something new in Guile.  Since the behavior of 'include' is not
standardized, I'd prefer we change it only if there are interesting use
cases not yet covered (can you think of a scenario?  we could add a test
for it).

> Also, mind the use of the fluid and how it interact with parallel compilation.

Fluids are thread safe, as far as I know, and files are compiled one at
a time anyway, so I don't foresee any problem here, as you also noted in
#scheme on Libera.

-- 
Thanks,
Maxim





  reply	other threads:[~2023-11-14 13:57 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-17  8:22 bug#66046: Relative includes in R7RS define-library seem broken Daphne Preston-Kendal
2023-11-06 18:31 ` Timothy Sample
2023-11-06 18:48   ` Maxim Cournoyer
2023-11-06 19:57     ` Maxim Cournoyer
2023-11-07  4:42     ` Maxim Cournoyer
2023-11-10  3:36       ` bug#66046: [PATCH 1/2] tests: Add new compile-file tests Maxim Cournoyer
2023-11-10  3:36         ` bug#66046: [PATCH 2/2] ice-9: Fix 'include' when used in compilation contexts Maxim Cournoyer
2023-11-11 11:58 ` bug#66046: Relative includes in R7RS define-library seem broken Amirouche
2023-11-14 13:57   ` Maxim Cournoyer [this message]
2023-11-18 22:56     ` Maxim Cournoyer
2023-11-22 16:11 ` bug#66046: [PATCH v2 1/3] libguile/fports.c: Remove extraneous include Maxim Cournoyer
2023-11-22 16:11   ` bug#66046: [PATCH v2 2/3] tests: Add new compile-file tests Maxim Cournoyer
2023-11-22 16:11   ` bug#66046: [PATCH v2 3/3] ice-9: Fix 'include' when used in compilation contexts Maxim Cournoyer
2023-11-22 16:17 ` bug#66046: [PATCH v3 1/3] libguile/fports.c: Remove extraneous include Maxim Cournoyer
2023-11-22 16:17   ` bug#66046: [PATCH v3 2/3] tests: Add new compile-file tests Maxim Cournoyer
2023-11-22 16:17   ` bug#66046: [PATCH v3 3/3] ice-9: Fix 'include' when used in compilation contexts Maxim Cournoyer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/guile/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87il64cu8z.fsf@gmail.com \
    --to=maxim.cournoyer@gmail.com \
    --cc=66046@debbugs.gnu.org \
    --cc=amirouche@hyper.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).