-
Notifications
You must be signed in to change notification settings - Fork 9
Add Support for Linux #1
Comments
👋 I just tried setting up a PR, and I think I'm running headfirst into this. The error I'm getting on
I have a sneaky suspicion there's some sort of thread race scenario happening here, but I'm no expert. |
I suspect the evaluationHandler on the REPL object isn't being called and finalized before you invoke the completion, even with I wonder if you don't want to explicitly force a "wait to submit the next evaluate" until the first has returned - or some other means of chaining these events explicitly - to force that evaluation ordering. (wishing for a little async/await here) |
@heckj Yeah, I've found myself missing proper async / await a lot lately. Though in this case, I think the REPL has a reasonable guarantee about I/O ordering — the only trick is to buffer output to avoid incorrect UTF-8 text segmentation. I think your assessment of why this isn't working on Linux is accurate; between the abstraction of |
I started poking at this over the weekend, and realized that my pattern for solving this would likely be using some sort of promise structure. Since there's not such a library in the standard library, and not one already in play here, do you have a preferred promise-like library that you'd be OK with using? I didn't want to propose adding a dependency without asking for preferences first. I did some light research, but no deep comparisons, and found quite a number of options, including two that looked interesting to me:
I also looked at PromiseKit, but since there was so a lot of ObjC pieces to it, I thought it might be best to leave it alone for such a swift-focused project. My own inclination (due to past familiarity with the API style emulated) is to the use Bluebird.swift, but that's entirely because it feels the most comfortable to me. |
I'm poking more into |
I've started a branch in my repo fork, but didn't want to open a PR until it moves beyond my stupid-trying-things-out-phase. Still, its public if you're interested in watching: https://github.com/heckj/DocTest/tree/linux. If you'd like it opened as a draft PR just to keep an eye, I'd be happy to do that. |
@heckj Awesome. Yeah, go ahead and submit that as a draft PR whenever you have a chance. That has a lot more visibility than a fork, which servers two purposes: 1) avoiding duplication of effort, and 2) providing an opportunity for others to share any useful insights they have. |
@mattt After a few rounds of digging and debugging, the underlying issue isn't with anything in the code at all, but pertains to Linux security (and docker) and the use of the REPL, which this tool invokes down into with it's subprocess pieces. Effectively using the REPL within a docker container requires loosening the permissions on the container while it's running, specifically with The PR that I added (#12) ends up not being needed at all, but was an interesting debugging exercise. So I think this might be resolved most simply by documenting that running this within a container requires looser security constraints - although how to wrap that into something like GitHub Action image and Short form that shows this working: Dockerfile
The run it through:
And within the docker container:
And with the looser constraints on the container, it all works. |
@heckj Thanks again for investigating this issue. I understand that this probably isn't the most satisfying resolution to the problem, but this is wonderfully useful information for us to share in the documentation for this project. Searching around based on your comment, I found this issue, which provides some more context: swiftlang/swift-docker#9 Combining that with the information from this thread on the GitHub forums, I was able to configure the CI action to work on Linux: https://github.com/SwiftDocOrg/DocTest/runs/630643726?check_suite_focus=true Edit: It looks like your action finished at the same time: https://github.com/SwiftDocOrg/DocTest/actions/runs/91427859 Comparing the logs for both, it looks like the workflow file in #17 is able to save a few minutes by avoiding the extra docker image build. Any objections to going with that solution instead? |
All good - I'm glad we found a resolution! That's what matters, and I learned a few interesting things along the way (including what a PITA it is to get LLDB to invoke tests on Linux ;-) ) I've closed my other branch, as it was mostly investigative |
(note - you'll notably speed up a local docker build if we have a .dockerignore file that excludes .git, .swiftpm, and .build if it exists - more related to anyone else coming along and debugging locally than a CI instance, which should be clean to start with) |
Tests are currently failing on Linux, although it's unclear exactly why. Perhaps something to do with a behavioral difference in the Swift REPL between macOS and Linux.
The text was updated successfully, but these errors were encountered: