search

Shen-Language / shen-cl

Shen for Common Lisp (Unmaintained)
BSD 3-Clause "New" or "Revised" License
122 stars 11 forks source link

Add BSD support #7

Closed cryptorick closed 6 years ago

cryptorick commented 6 years ago

Hi!

With these changes, Shen installs a bit easier for FreeBSD, OpenBSD and NetBSD users. A few notes.

rkoeninger commented 6 years ago

@cryptorick Thanks! Getting Shen working reliably on BSD will be a boon to the language.

I tested this branch on Windows and it works fine, and the CI build passes, so we know it works on recent Linux (Ubuntu).

One little thing: Clozure CL is not mentioned in the readme additions under the BSDs. Ideally, we'd have each CL implementation tested per OS and mention explicitly in the readme which are not working. Is Clozure well-supported on BSD or installable in the same way as the others? Does gmake ccl work?

I'll also wait for @tizoc review it before hitting merge. And maybe try it out on macOS.

tizoc commented 6 years ago

Thank you @cryptorick !

The changes look good to me and safe to merge. I can't test it on OSX today, but I will do it during the weekend and report back.

cryptorick commented 6 years ago

BTW, here's a rundown of available CLs on these BSDs (as of today), for which I will change the All variable accordingly (as you requested).

I was surprised to find out that I couldn't find a (binary) package of clisp on FreeBSD, because I used to have it; it turns out that the port/package was deprecated. This usually happens when a (volunteer) port maintainer can't continue anymore (and no one else was found to take over). OTOH, FreeBSD is the only BSD here to have ccl (a very nice CL).

cryptorick commented 6 years ago

(Sorry for the double push -- I was working with too many locals and forgot where I was. :)

@rkoeninger @tizoc I think I covered all the outstanding items, but please check me.

Also, you wouldn't put me off in the least of you changed anything I submitted to improve the expression or fix something functionally. I just want it to work, and I consider this a team effort.

That said, the text I typed in the README may be a bit too long-winded / not in keeping with what you have in the other sibling sections. Please change to whatever seems more appropriate for your style and your audience (whom you know better than I do anyway).

Also, I'm not a fan of -- and no great shakes at -- writing conditionals in GNU makefiles. For instance, the lines I added, namely:

    ifeq ($(OSName),freebsd)
        All=ccl ecl sbcl
    else ifeq ($(OSName),openbsd)
        All=clisp ecl sbcl
    else ifeq ($(OSName),netbsd)
        All=clisp ecl
    endif

seem to work fine (I tested it); however, according to the manual here which says

Extra spaces are allowed and ignored at the beginning of the conditional directive line, but a tab is not allowed. (If the line begins with a tab, it will be considered part of a recipe for a rule.)

the tab I put in front of ifeq ($(OSName),freebsd) (on the first line) should be a no-go; yet, it works. (Maybe this is because, at this point, we are not yet under a rule?)

But also, that block I inserted should end with two more endifs, according to the manual (same section), but ... it works. However, I don't usually write GNU makefiles; so, I am not sure that it is correct and I hope you will please check it out. Thanks! You guys have been great. I wish this project great success!

rkoeninger commented 6 years ago

@cryptorick I remember avoiding indenting control structures, but when I removed them, CI failed and I don't feel like playing 20 questions with it.

I added a grid of install instructions/compatibilities for the BSDs to the readme (based on this). I don't know the BSD package commands. Is it the same for all of them? If the commands are the same, then the grid can just mention compatibilities.

Let's can go ahead and wrap this up, and I'll worry about neat presentation later.

cryptorick commented 6 years ago

@rkoeninger I added the package manager commands to the matrix. I hope that's what you wanted. I will test the current Makefile, with your changes, next. (Roger on the 20 questions.)

cryptorick commented 6 years ago

Oops. End of gmake invocation.

;; Wrote the memory image into ./bin/clisp/shen (8,856,736 bytes)
Bytes permanently allocated:            164,192
Bytes currently in use:               4,996,752
Bytes available until next GC:           77,296
Bye.
./bin/clisp/shen --clisp-m 10MB -e "(do (cd \"kernel/tests\") (load \"README.shen\") (load \"tests.shen\"))"
GNU CLISP: invalid argument: '-e'
GNU CLISP: use '-h' for help
gmake: *** [Makefile:150: test-clisp] Error 1

Don't know why it doesn't like -e. Any ideas?

I looked at the Clisp --help and it doesn't list -e, but lists -x as the option to eval expressions. But that doesn't seem to work either:

$ ./bin/clisp/shen --clisp-m 10MB -x "(do (cd \"kernel/tests\") (load \"README.shen\") (load \"tests.shen\"))" 
module 'syscalls' requires package OS.

This is on OpenBSD 6.2 amd64. gmake sbcl and gmake ecl are both completing successfully though.

rkoeninger commented 6 years ago

@cryptorick I think the readme is good now. I started going overboard on how much instruction was provided to the user. The CL's individual project pages can tell them anything more they need to know.

The -e argument is supposed to be handled by shen.toplevel in primitives.lsp, but it looks like clisp itself is trying to interpret it. I thought arguments to clisp itself had to be prefixed with --clisp-, like --clisp-m. That's the way it works everywhere else.

Was this working earlier? Also, you should be able to run make all/gmake all successfully on a machine with all available CL binary distributions installed.

cryptorick commented 6 years ago

Was this working earlier?

No. I had only run gmake sbcl on OpenBSD earlier (cf. my original message).

cryptorick commented 6 years ago

Status on builds/tests after your makefile changes

The NetBSD box I'm on is a shell account for which I've put in a request to the admin (today) to install ecl. I will report back when that happens. If it takes too long, I could find another machine in a few days.

(BTW, as you have probably guessed, that odd error on OpenBSD cannot be caused by any changes we made to Makefile, since I get the same error when I switch back to the master branch.)

cryptorick commented 6 years ago

My sysadmin installed ecl on the NetBSD machine. All's well. So now, the status of the builds/tests are:

Still don't know what to do about that OpenBSD Clisp -e thingy yet. Until we resolve that issue, in the meantime, should we take clisp off of the All list for OpenBSD, or leave it the way it is? Thanks!

rkoeninger commented 6 years ago

@cryptorick I went ahead and removed clisp from the All variable for OpenBSD. I installed OpenBSD in a VM last night and was able to reproduce the problems you mention, but I have no idea what to do about them.

The make all command is really just a convenience for testing changes - I wanted to be able to re-build and re-test all CLs on a machine quickly. So a CL missing from the All variable doesn't guarantee it won't work, it's just not been shown to consistently work yet.

I think we've gotten plenty done with this PR and we can put the OpenBSD/CLisp problems in as an issue if we want to revisit them later. I'll merge it in unless there are any other issues.

Thanks for contributing and all your help with testing!

cryptorick commented 6 years ago

@rkoeninger sounds great! Thanks!

cryptorick commented 6 years ago

@rkoeninger sorry you had to go to the trouble of standing up an OpenBSD VM. Glad you were able to do it and reproduce the problem. ("I'm not crazy!" \o/ :)