justinethier / cyclone

:cyclone: A brand-new compiler that allows practical application development using R7RS Scheme. We provide modern features and a stable system capable of generating fast native binaries.
http://justinethier.github.io/cyclone/
MIT License
823 stars 42 forks source link

A library's `include` does not consider directories provided with the flag -I #494

Closed amirouche closed 1 year ago

amirouche commented 1 year ago

Here is an example checks.scm program, that rely on a library (united base) that includes body.scm:

;; cat checks.scm 
(import (scheme base))
(import (united base))

(define checks (quasiquote ((((united base) ~check-united-base-000) unquote ~check-united-base-000) (((united base) ~check-united-base-001) unquote ~check-united-base-001))))

(display "* ")(display "cyclone")
(newline)

(let loop ((checks checks))
  (unless (null? checks)
    (display "** ")
    (display (caar checks))
    (newline)
    ((cdar checks))
    (loop (cdr checks))))

;; cat tests/united/base.sld 
(define-library (united base)
  (export display newline command-line
          ~check-united-base-000
          ~check-united-base-001
          )
  (import (scheme base)
          (scheme write)
          (scheme process-context))

  (include "body.scm"))

;; cat tests/united/body.scm 
(define ~check-united-base-000
  (lambda ()
    (display "youhou one\n")
    #t))

(define ~check-united-base-001
  (lambda ()
    (display "youhou two\n")
    #t))

Then try to run checks.scm yields the following error:

;; /home/amirouche/.local/opt/united//cyclone/bin/cyclone -I tests/ checks.scm
Error: Unable to open file: 
".//united/body.scm"

Error: Unable to compile library: 
(united base)

;;
amirouche commented 1 year ago

If I change directory to ./tests/ and adjust the command, it works.

lassik commented 1 year ago

AFAIK -I is designed to affect only import, not include. At least SRFI 138 says so.

lassik commented 1 year ago

Can you do it the other way around so the files in tests/ import the wrapper? Or maybe load would work.

amirouche commented 1 year ago

AFAIK -I is designed to affect only import, not include. At least SRFI 138 says so.

Yes, it does:

-I directory Prepend directory to the list of directories that are searched in order to locate imported libraries. https://srfi.schemers.org/srfi-138/srfi-138.html

And then R7RS small, says the following:

Inclusion

(include string ...) syntax
(include-ci sitring ... *) syntax

Semantics: Both include and include-ci take one or more filenames expressed as string literals, apply an implementation-specific algorithm to find corresponding files, read the contents of the files in the specified order as if by repeated applications of read, and effectively replace the include or include-ci expression with a begin expression containing what was read from the files. The difference between the two is that include-ci reads each file as if it began with the #!fold-case directive, while include does not.

The above paragraph is immediatly followed with the following note:

Note: Implementations are encouraged to search for files in the directory which contains the including file, and to provide a way for users to specify other directories to search.

And the behavior in that note is implemented by chicken, chez (racket, and cisco), gauche, gambit, chibi; Guile has a bug report, that includes a workaround.

amirouche commented 1 year ago

Can you do it the other way around so the files in tests/ import the wrapper?

No. I use an include to share code across R6RS, R7RS, and Racket.

maybe load would work.

I will fallback to load if it works, if there is no way to improve include.

Note: That code looks interesting: https://github.com/justinethier/cyclone/blob/5e509495befc211d88285a7d4e377a9174df0690/cyclone.scm

lassik commented 1 year ago

-I directory Prepend directory to the list of directories that are searched in order to locate imported libraries. https://srfi.schemers.org/srfi-138/srfi-138.html

That part is talking about imported libraries, not included files. (But maybe it's intended to apply to included files too.)

And the behavior in that note is implemented by chicken, chez (racket, and cisco), gauche, gambit, chibi; Guile has a bug report, that includes a workaround.

Wow, I had no idea! Thank you for surveying it.

justinethier commented 1 year ago

Thanks for the bug report!

Note you can work around this by compiling the library directly:

$ cyclone -I tests/ tests/united/base.sld 
$ cyclone -I tests/ checks.scm 
justinethier commented 1 year ago

Alright, this is properly fixed now on the master branch.

amirouche commented 1 year ago

It works. Thanks!

amirouche commented 1 year ago

It looks like it is not fixed... In any case, the include should work relative to the including .sld, so it should indeed take into account -I and -A and also know about the directory where the .sld is.

~I will try to fix it :)~ Feel free to pick up the issue, see the comment below.

amirouche commented 1 year ago

Sorry, I am prolly confusing. I found the problem, that might have been the problem from the start of this whole issue, it is related to cond-expand: lib:includes does not consider include inside cond-expand, here is a test case:

;; #> cat tmp/main.scm tmp/foobar/base.sld tmp/foobar/body.scm 
(import (scheme base))
(import (scheme write))
(import (foobar base))

(display foobar)(newline)
(define-library (foobar base)
  (export foobar)
  (import (scheme base))

  (cond-expand
   (cyclone
    (include "body.scm"))))
(define foobar "win")
;; #> ./cyclone -A tmp/ tmp/main.scm 
Error: Unable to open file: 
"body.scm"

;; #> 
amirouche commented 1 year ago

IIUC, https://github.com/justinethier/cyclone/issues/494 allows include to look inside directories provided via -I and -A e.g. cyclone -A my/path/ will suggest (include "something.scm") to try to open my/path/something.scm.

It looks useful, tho, I have never seen include used like that. In my case, and what other implementations do is the following:

Given /path/to/foo/bar.sld that looks like:

(define-library (foo bar)
  (import (scheme base))
  (export foobar)

  (cond-expand
    (cyclone (include "body.scm")))

The include, should consider (first?) the following file: /path/to/foo/body.scm