Skip to content

Fix Linux compatibility issues#1534

Closed
abdimaye wants to merge 12 commits intoapollographql:mainfrom
abdimaye:fix-linux-compatibility-issues
Closed

Fix Linux compatibility issues#1534
abdimaye wants to merge 12 commits intoapollographql:mainfrom
abdimaye:fix-linux-compatibility-issues

Conversation

@abdimaye
Copy link
Copy Markdown

@abdimaye abdimaye commented Nov 23, 2020

Issue #1506

List of changes:

@apollo-cla
Copy link
Copy Markdown

@abdimaye: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/

@abdimaye abdimaye marked this pull request as draft November 23, 2020 17:51
@designatednerd
Copy link
Copy Markdown
Contributor

@abdimaye can you elaborate a bit on what's still not working?

@designatednerd designatednerd added the Linux Issues specific to use (or at least attempted use) on Linux label Nov 23, 2020
@abdimaye
Copy link
Copy Markdown
Author

@abdimaye can you elaborate a bit on what's still not working?

Planning to add the details to the list of outstanding errors later today 👍

@designatednerd
Copy link
Copy Markdown
Contributor

Some thoughts that might help looking over those errors:

  • I'm pretty sure for Bundle+Helpers you can just throw an if not linux around the contents of that file since it deals with Bundle stuff that I'm almost positive doesn't exist in the Linux version of Foundation.
  • Is URLRequest not one of the things that's in FoundationNetworking? That's a significant surprise if not.
  • I think for the URLSessionTaskMetrics and URLSessionStreamTask you should be able to use an unavailable marker for linux on the enclosing methods.
  • Not sure what Foundation for Linux has got to figure out the string encoding, but you'd probably need to do something similar to what you're doing with Crypt to create a protocol to handle those
  • Not sure what the hell is going on with the JSON error, tbh.

Comment thread Sources/Apollo/StringEncoding.swift Outdated
@abdimaye
Copy link
Copy Markdown
Author

@designatednerd sorry I've been ghost! I think the encoding issue was the last. Looks like there are some merge conflicts also which I'll take a peek at later.

@abdimaye abdimaye marked this pull request as ready for review December 17, 2020 14:58
@designatednerd
Copy link
Copy Markdown
Contributor

Awesome work - though I'm a bit confused as to why the tests aren't running on this at all. I'll poke at it this afternoon.

@designatednerd
Copy link
Copy Markdown
Contributor

I'd also like to know if you have suggestions for at least making sure things continue to build on linux - what linux setup should we be using for tests, etc

@designatednerd
Copy link
Copy Markdown
Contributor

I wonder if it wasn't building because this was a draft from a forked repo - can you make another commit please?

@abdimaye
Copy link
Copy Markdown
Author

I'd also like to know if you have suggestions for at least making sure things continue to build on linux - what linux setup should we be using for tests, etc

I've now added a Dockerfile and build script for linux. Not entirely sure about the testing as I'm still trying to figure this out...

@designatednerd
Copy link
Copy Markdown
Contributor

Taking a stab at adding a linux build to the matrix...apologies for pushing to your branch but it's not going to work on any other branch at the moment 😛

@designatednerd
Copy link
Copy Markdown
Contributor

Woof. Running into some serious issues around the fact that even though we're specifically saying only import Crypto on linux, it's still saying it's a dependency on macOSX. I have no idea why this wasn't failing the build before - I'm going to try to do some swift 5.3 stuff on a separate branch to try and disentangle a bunch of changes I need to make to file structure because of resources from this PR

…onflicts

# Conflicts:
#	Apollo.xcodeproj/project.pbxproj
#	Package.swift
@designatednerd
Copy link
Copy Markdown
Contributor

Looks like the SQlite sub-lib is not compiling on linux since it uses a bunch of stuff which is iOS specific. Ugh, I wish platform restrictions per target were a thing.

I'll have to reach out to the swift forums about why things are trying to import Crypto lib even though it specifically says it should only be brought in on linux

@abdimaye
Copy link
Copy Markdown
Author

Looks like the SQlite sub-lib is not compiling on linux since it uses a bunch of stuff which is iOS specific. Ugh, I wish platform restrictions per target were a thing.

I'll have to reach out to the swift forums about why things are trying to import Crypto lib even though it specifically says it should only be brought in on linux

😕 Would it be reasonable to skip any tests which have platform specific dependencies for this first iteration?

@designatednerd
Copy link
Copy Markdown
Contributor

Unfortunately the problem is not that the tests won't run, it's that the libraries won't even build. Again, I'm not sure why this wasn't showing up as a problem earlier. 😭

@abdimaye
Copy link
Copy Markdown
Author

Have you tried xcodebuild? Something like xcodebuild -scheme Apollo?

@designatednerd
Copy link
Copy Markdown
Contributor

That's what we're doing on the iOS and mac tests already, unfortuantely. And it's still giving us problems.

@abdimaye
Copy link
Copy Markdown
Author

abdimaye commented Jan 6, 2021

Found another issue that's kind of related to this - type checking queryDocument would timeout on linux. Here's the PR from my colleague: apollographql/apollo-tooling#2198

@abdimaye
Copy link
Copy Markdown
Author

Completely forgot about this! I won't be looking into this further so closing PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Linux Issues specific to use (or at least attempted use) on Linux

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants