Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Port to Gerbil Scheme #314

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Port to Gerbil Scheme #314

wants to merge 4 commits into from

Conversation

vyzo
Copy link

@vyzo vyzo commented Mar 23, 2018

This adds support for Gerbil in the Scheme implementation.

@kanaka
Copy link
Owner

kanaka commented Mar 23, 2018

@vyzo can you add the installation of gerbil to the Dockerfile and add that mode to .travis.yml to activate Travis testing? Once you do that I'll rebuild and push the docker image and restart the Travis test.

@vyzo
Copy link
Author

vyzo commented Mar 23, 2018

sure!

@vyzo
Copy link
Author

vyzo commented Mar 23, 2018

It seems the test failed because gxc not found. Have you updated the docker container?
Perhaps I should install in /bin instead of /gerbil/bin and adding to path in the env.

@vyzo
Copy link
Author

vyzo commented Mar 23, 2018

I updated the Dockerfile to install gerbil in /opt/gerbil.

@wasamasa
Copy link
Collaborator

@vyzo I planned to wait with this one until Gerbil scheme has been properly packaged so that no installation to /opt or alike is needed and binaries would end up in /usr/bin like with everything else.

  • The gerbil-build.sh thing looks mostly useless. Why not make it part of the Makefile? Instead of building everything at the first step, you build/rebuild stuff as needed. The existing definitions show how it's done for other Scheme implementations. Note that $(RM) -rf isn't needed, it's covered by $(RMR).
  • What's the deal with (export main) and the later (interactive-main)? Is there a way to disable this special interpretation of the main procedure? Do you really have to rename the main function because it has a different function signature from the exported main procedure?

@vyzo
Copy link
Author

vyzo commented Mar 23, 2018

I can change the Dockerfile to install on /usr/bin, that's not such a difficult thing.

  • The gerbil-build.sh script is used to build because the gerbil compiler output doesn't match the 1-to-1 model of Makefiles; there are several artifacts produced by the compiler for each module and it was much simpler for me to just have a script than trying to fight with the Makefile. Sorry, my Makefile fu is not strong enough.

  • Gerbil requires a main function as entry point for executables, which receives the command line arguments. There is (currently) no way to disable this behaviour, which is desirable for several reasons (and you can also argue that it's undesirable for some other reasons).

@kanaka
Copy link
Owner

kanaka commented Apr 2, 2018

@vyzo I pushed out an updated build of the scheme docker image and restarted the gerbil task. There are some errors during the build phase and also seem to be some ASCII control characters being printed in the output that are breaking the tests: https://travis-ci.org/kanaka/mal/jobs/357610551

@vyzo
Copy link
Author

vyzo commented Apr 2, 2018

seems my build script is not expanding the lib/{util,types,env,reader,printer,core}.sld construct -- probably because the underlying shell is not bash.
Let me try to fix it.

@vyzo
Copy link
Author

vyzo commented Apr 2, 2018

not sure what's up with the control characters though.

@vyzo
Copy link
Author

vyzo commented Apr 2, 2018

I also recently introduced a bug in Gerbil r7rs, which is now fixed:
mighty-gerbils/gerbil@aa051ea

We will need to build a new docker container for that fix, as it breaks mal compilation.
Sorry about that.

@kanaka
Copy link
Owner

kanaka commented Mar 15, 2019

Hi @vyzo, are you still interested in pursuing this? I built and pushed a new scheme docker image that updates the gerbil repo as of today it's state a couple hours ago. The first thing I ran into is that the build rule dependencies aren't complete. Here is a patch to my tree that fixes that (and also moves the gerbil-build.sh logic into the Makefile):

diff --git a/Makefile b/Makefile
index d88cd89..0ba8e49 100644
--- a/Makefile
+++ b/Makefile
@@ -169,6 +169,7 @@ scheme_STEP_TO_PROG_chicken     = scheme/$($(1))
 scheme_STEP_TO_PROG_sagittarius = scheme/$($(1)).scm
 scheme_STEP_TO_PROG_cyclone     = scheme/$($(1))
 scheme_STEP_TO_PROG_foment      = scheme/$($(1)).scm
+scheme_STEP_TO_PROG_gerbil      = scheme/gxc.out/bin/$($(1))
 
 # Map of step (e.g. "step8") to executable file for that step
 ada_STEP_TO_PROG =     ada/$($(1))
diff --git a/scheme/Makefile b/scheme/Makefile
index ccb719e..94b86f5 100644
--- a/scheme/Makefile
+++ b/scheme/Makefile
@@ -9,11 +9,11 @@ CLASSSTEPS = out/step0_repl.class out/step1_read_print.class \
              out/step3_env.class out/step4_if_fn_do.class out/step5_tco.class \
              out/step6_file.class out/step7_quote.class out/step8_macros.class \
              out/step9_try.class out/stepA_mal.class
-GXCSTEPS = gxc.out
+GXCSTEPS = $(foreach s,$(BINS),gxc.out/bin/$(s))
 STEPS = $(if $(filter kawa,$(scheme_MODE)),$(CLASSSTEPS),\
         $(if $(filter chicken,$(scheme_MODE)),$(BINS),\
         $(if $(filter cyclone,$(scheme_MODE)),$(BINS),\
-               $(if $(filter gerbil,$(scheme_MODE)),$(GXCSTEPS)))))
+        $(if $(filter gerbil,$(scheme_MODE)),$(GXCSTEPS)))))
 
 KAWA_STEP1_DEPS = out/lib/util.class out/lib/reader.class \
                   out/lib/printer.class out/lib/types.class
@@ -71,8 +71,12 @@ eggs/r7rs.so:
        chicken-install -init eggs
        CHICKEN_REPOSITORY=$(CURDIR)/eggs chicken-install r7rs
 
-gxc.out:
-       ./gerbil-build.sh
+gxc.out/lib/core__rt.ol:
+       mkdir -p gxc.out/bin
+       GERBIL_LOADPATH=gxc.out gxc -d gxc.out -O lib/util.sld lib/types.sld lib/env.sld lib/reader.sld lib/printer.sld lib/core.sld
+
+gxc.out/bin/%: gxc.out/lib/core__rt.ol
+       GERBIL_LOADPATH=gxc.out gxc -d gxc.out -O -exe -o $@ $*.scm
 
 lib/%.scm: lib/%.sld
        $(SYMLINK) $< $@

Once that is applied, the steps will build and the tests can run but then I see the original failure I was seeing where there is escape codes in the output. Here is a snippit of the test failure (test^step0):

Testing basic string
TEST: abcABC123 -> ['',abcABC123] -> FAIL (line 3):
    Expected : 'abcABC123\r\nabcABC123'
    Got      : '\x1b[1mabcABC123\x1b[m\r\nabcABC123'
Testing string containing spaces
TEST: hello mal world -> ['',hello mal world] -> FAIL (line 7):
    Expected : 'hello mal world\r\nhello mal world'
    Got      : '\x1b[1mhello mal world\x1b[m\r\nhello mal world'

If you're still interested can you fix the failures? Also, since @wasamasa created the base scheme implementation he'll either need to sign off on the way you are doing the main invocation, or you'll need to bring that more into line with the way all the other scheme implementations currently work.

If you're not interested in pushing this forward, please let me know and I'll close the issue. Thanks!

@vyzo
Copy link
Author

vyzo commented Mar 16, 2019

I can look into fixing the test failures.

@vyzo
Copy link
Author

vyzo commented Mar 16, 2019

Seems like output is getting garbled with control characters.

@kanaka
Copy link
Owner

kanaka commented Mar 18, 2019

I suspect it's whatever library you are using to do line editing (e.g. readline, libedit, linenoise) is trying to do some sort of input highlighting or cursor movement using escape codes.

Copy link
Owner

@kanaka kanaka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vyzo Any interest in revisiting this and getting it merged?

If so, you'll want to do a few things since so much time has passed:

  • rebase onto the HEAD sources
  • move the .travis.yml definition to IMPLS.yml
  • Make sure that the Github CI tests all pass for your implementation. The process is now more automated and doesn't require my manual intervention to build docker images (part of the reason to rebase).

Thanks!

@vyzo
Copy link
Author

vyzo commented Aug 7, 2024

Sure! It's been a while, and Gerbil has evolved a lot since, but it could be a fun exercise.

Time is not ample unfortunately, but i'll put it in my list of things to hack.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants