Fix #667 equality tests in elisp and powershell #669
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kanaka
requested changes
Aug 26, 2024
impls/powershell/Dockerfile
Outdated
| #RUN curl -L -O https://github.com/PowerShell/PowerShell/releases/download/v6.0.0-alpha.9/powershell_6.0.0-alpha.9-1ubuntu1.16.04.1_amd64.deb && \ | ||
| # dpkg -i powershell_6.0.0-alpha.9-1ubuntu1.16.04.1_amd64.deb && \ | ||
| # rm powershell_6.0.0-alpha.9-1ubuntu1.16.04.1_amd64.deb | ||
| RUN apt-get-y install ca-certificates curl |
There was a problem hiding this comment.
Looks like missing space ("apt-get-y" -> "apt-get -y") causing the image build to fail. I experimented a bit trying to build the image with the updated powershell. Seems like the powershell deb doesn't quite support 24.04 yet (due to dep on libicu that doesn't include the version in 24.04). Forcing the install seems to allow powershell to install and run:
diff --git a/impls/powershell/Dockerfile b/impls/powershell/Dockerfile
index e7e957e5..365881a6 100644
--- a/impls/powershell/Dockerfile
+++ b/impls/powershell/Dockerfile
@@ -19,10 +19,10 @@ WORKDIR /mal
# Specific implementation requirements
##########################################################
-RUN apt-get-y install ca-certificates curl
+RUN apt-get -y install ca-certificates curl libicu74
RUN curl -L -O \
https://github.com/PowerShell/PowerShell/releases/download/v7.4.5/powershell_7.4.5-1.deb_amd64.deb && \
- dpkg -i powershell_7.4.5-1.deb_amd64.deb && \
+ dpkg -i --force-depends powershell_7.4.5-1.deb_amd64.deb && \
rm powershell_7.4.5-1.deb_amd64.deb
I think using Ubuntu 22.04 and libicu72 also seems to work.
In either case, when I try and use that to test the current state of this branch, it seems to fail in step1 when processing a "+" sign:
user> +
An error has occurred that was not properly handled. Additional information is shown below. The PowerShell process will exit.
Unhandled exception. System.NullReferenceException: Object reference not set to an instance of an object.
at CallSite.Target(Closure, CallSite, Object, ExecutionContext)
Reverting to the original version of the image that has powershell 6.0.0-alpha.9, all tests pass with your code changes. You have to use the original upstream docker image (because vivid image can't be rebuilt any more).
make impl_to_image=ghcr.io/kanaka/mal-test-powershell:20200211_055016-g8a19f603 DOCKERIZE=1 test^powershell
Looks like Ubuntu 16.04 packages are still available, so the following would allow the image to be rebuilt and seems to still pass all the tests:
diff --git a/impls/powershell/Dockerfile b/impls/powershell/Dockerfile
index e7e957e5..1ecddb9d 100644
--- a/impls/powershell/Dockerfile
+++ b/impls/powershell/Dockerfile
@@ -1,4 +1,4 @@
-FROM ubuntu:24.04
+FROM ubuntu:16.04
MAINTAINER Joel Martin <[email protected]>
##########################################################
@@ -9,8 +9,10 @@ MAINTAINER Joel Martin <[email protected]>
RUN apt-get -y update
# Required for running tests
-RUN apt-get -y install make python3
-RUN ln -fs /usr/bin/python3 /usr/local/bin/python
+RUN apt-get -y install make python
+
+# Some typical implementation and test requirements
+RUN apt-get -y install curl libreadline-dev libedit-dev
RUN mkdir -p /mal
WORKDIR /mal
@@ -19,10 +21,12 @@ WORKDIR /mal
# Specific implementation requirements
##########################################################
-RUN apt-get-y install ca-certificates curl
-RUN curl -L -O \
- https://github.com/PowerShell/PowerShell/releases/download/v7.4.5/powershell_7.4.5-1.deb_amd64.deb && \
- dpkg -i powershell_7.4.5-1.deb_amd64.deb && \
- rm powershell_7.4.5-1.deb_amd64.deb
+# Nothing additional needed for python
+RUN apt-get -y install libunwind8 libicu55
+
+# For dist packaging
+RUN curl -L -O https://github.com/PowerShell/PowerShell/releases/download/v6.0.0-alpha.9/powershell_6.0.0-alpha.9-1ubuntu1.16.04.1_amd64.deb && \
+ dpkg -i powershell_6.0.0-alpha.9-1ubuntu1.16.04.1_amd64.deb && \
+ rm powershell_6.0.0-alpha.9-1ubuntu1.16.04.1_amd64.deb
ENV HOME=/mal
The original motivation is to fix the new (= nil ()) and core_apply_accepts_macros tests. Improve speed and warnings with byte compilation. mal/core.el: Wrap core functions during the loop in main, instead of writing the conversion in each line of core-ns. Use apply built-in concatenation of last argument. Move handling of metadata to types.el. mal/env.el: Represent environments as cons cells instead of vectors. mal/func.el: Merged into types.el, it is not a special case anymore. mal/printer.el: Add macro case. Define a pr-join helper for sequences and core.el. mal/reader.el: Rename the tokens local variable in reader.el (compiler warning). mal/types.el: Use type-specific accessors returning nil for the wrong type (structural pattern matching would be better, but is too slow). Represent native types directly when possible, and inline some trivial accessors. Use dedicated records instead of vectors. Implement metadata only when required. Represent keywords as strings (easyer, no counterpart). run: Run byte-compiled version. steps: Backport good ideas from stepA to step1, reducing the diff between steps for future maintenance. Implement 'do with a simple iteration (without map and butlast). Make the repl-env local to main (compiler warning). Make the code more idiomatic * prefer loop over recursion (search in environments) * declare variable and reassign them when convenient (exit of the TCO loop) * car cdr cadr and so on instead of nth * remove various vector <-> list conversions.
Fix (false? 1) and (true? 0).
Change the type of the env.{find,get,set} key to a raw string.
Merge eval-ast into EVAL.
Implement DEBUG-EVAL.
Prevent defmacro! from mutating the original function.
asarhaddon
force-pushed
the
elisp-powershell
branch
from
b8e124a to
0cf6724
Compare
29 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes #667, and improves the elisp implementation.