-
Notifications
You must be signed in to change notification settings - Fork 16
[WIP] Provide Support for Linux #36
base: master
Are you sure you want to change the base?
Conversation
c911707
to
13e9ece
Compare
What does |
No, it is actually a part of Foundation and it’s not currently implemented. We’ll have to do something about it, unless the corelibs team surprises us with a patch that implements that. |
swiftlang/swift-corelibs-foundation#1427 The author should stop being lazy and finish the requested feedback. 😏 |
Omg, I didn't realise you were working on that! Yes, I agree 😜 but also I think his pace might be different. We should not be pressuring him :) |
How're we doing logging? Because tracelog is an option to have cross-platform logging :) |
PS: I edited the original to-do list so that it's easier to follow, how's it look? |
LGTM 👍
So I will be honest and say I haven't put too much thought into logging. So I'll try and stop this from becoming a stream of consciousness. My two platforms I daily drive are macOS and Fedora. On macOS I want the logging to be done using Unified logging or The thing that we absolutely cannot do is write to Other than that, I think its probably best to just try some out and see what happens. |
LSP also defines window/logMessage |
Awesome :)
That seems reasonable. So looking at TraceLog's Readme (under Configuration), I think we can definitely avoid writing to
|
Ohh, that is very interesting. Maybe we can pipe the logs to that eventually, or some of them at least (the ones relevant to have on the client's side) Is this the kind of logging you meant in the to-do list, Ryan? (I know very little about the LSP so I couldn't say) |
Absolutely I think that's a good one too. Traditionally I didn't use that one because when I was first starting this thing I really wanted to be able to log the raw bytes of the protocol. So if you were doing that over the So for a long time I was just logging every new message into a different file. Eventually this became too noisy and I fell back to This struck a nice balance between being able to see all the low-level logs from the server and also giving an easy filter mechanism. I still think that window/logMessage has value. Though right now the challenge is what are messages that are for developers of the LSP and what are user facing log messages. Also, if you look at the main run loop. It is pretty poorly structured to handle injecting out of band messages. The main run loop is basically designed to receive a request from the client and respond. It is not really designed to just say something without being prompted first. That's a design flaw by me.
No the logging I want to fix in the todo list is this. |
It's not like this was another one amongst the tens of LSP implementations for Swift. This was the first one. I wouldn't blame you for having a design mistake in something that was not only entirely new, but also very much urgently needed. Maybe the design could be reworked after everything is cross-platform and stable.
I see. I'll read the docs of TraceLog in more detail, I think it could work for this. |
Update on TraceLog:
Is very much possible :D
|
OMG! :D YOU CAN DOOO EEEET ✊ ...how did you get past the Foundation problems? Are you sneakily using your corelibs PR? :) |
Yup. I've compiled my own version of Swift 4.2. I'm using using my patch(es) and a few others that I've not published yet. I'm compiling swift, swift-stdlib and libdispatch in debug mode so I can actually use I've actually refactored the run loop to not use the old patch needed for Foundation. It now just uses The bad news is that now it depends on this patch. So I just traded one issue in corelibs for another I suppose. 🤷♂️ 🤦♂️ |
Wow. Having fiddled a little bit with compiling the toolchain, I know that's a ton of work. 🙇♂️ thank you
Well, that patch looks certainly much, much simpler. So that's great! It should be merged soonish, I'd say. The performance gains are great as well. Good job! ^^ |
66483d7
to
38c5442
Compare
I've updated the to-do list to reflect your new PR 👍 |
c1ba9bc
to
1a743f4
Compare
… name was different, now they are the same. There are 3 FIXMEs in the `LinuxMain.swift` file. Those are related to test files that make the test suite crash.
Added guard statements to replace forced unwraps that would otherwise crash Linux when running certain tests. Now that those tests are safe, I've enabled them on Linux. Now test coverage is complete on Linux.
1a743f4
to
268526d
Compare
@felix91gr the cross platform logging wasn't as straight forward as we might have hoped I take it. Swift on Linux is not for the faint of heart. 😏 🤣 |
Hahaha, I know right? Ah well. I'm gonna see if I can persuade somebody who's at WWDC s.t. they can get a word in for us for expanding the
Last year it was oh so much worse 🤣 but it's getting better! And thanks to people like you and @vknabel, among many others, we can push it forward even faster :). IMO the most important part of a project like Swift is the community behind it, and we have a really welcoming one, with very kind people all around. I've also seen great disposition on the |
I don't know if I have said or not but I've hit a road block since I cannot actually debug the server on Linux the same way I do on macOS. I'm now learning how LLDB works and trying (so far unsuccessfully) to implement the capability on Linux. 🤞
Could not agree more.
That aligns with my experience too. Keep up the great work! |
I've seen your quest for LLDB at the forums. I'm crossing my fingers for you as well!
You too! ^^ |
Following |
@RLovelett do you need assistance adding TraceLog and AdaptiveWriter for "Cross-platform logging"? I should have some time shortly and would be happy to assist when I do. |
Absolutely I'd love the assistance. @felix91gr was the one for the original "vision" of how to do this feature but certainly I can help answer questions. If you have a vision for it by all means move forward with it. |
@RLovelett Ok, I suggest that TraceLog is added in a branch off of |
Totally agree. That is a great idea. Especially love the idea of it being done directly off of master. Easier to review and move forward. |
Great, as soon as I free up a bit, I'll get setup with your dev environment, create the branch and get started. |
Infraestructure
Code
Ogra Error Message
corelibs-libdispatch
Documentation
README