-
-
Notifications
You must be signed in to change notification settings - Fork 319
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Setting up custom derived data #388
Comments
Hi, I'm interested in seeing if Bazel can be supported but only the standard and relative derived data paths are supported when using injection. Perhaps you could try using the https://github.com/johnno1962/HotReloading project which finds DerivedData wherever you put it. |
Let me take a look at https://github.com/johnno1962/HotReloading. Also I'm curious, since in https://github.com/johnno1962/HotReloading we could use custom DerivedData, in theory, if we swap the implementation in Injection III, with HotReloading ones to find DerivedData folder, it would work, isn't it? If so, which file I should look into? |
Start with HotReloading as it always finds the DerivedData folder using the package SwiftTrace and is easier to debug and iterate over than the app. I'm considering changing the error message you first encountered which almost reads like an invitation to use a custom derived data path which is difficult to support for the app. |
There, I changed the message, 0107904c2a7b96f91f50da4f126d8a0b43888865, 4.3.7 |
Hello @johnno1962, First thanks very much for this awesome project. Recently we are trying out this project to boost our productivity and we met the same issue with bazel. After some digging we found that the root cause for why InjectionIII is not working with bazel is not really the path change of Bazel do not follow the legacy Xcode One more thing, bazel by default do not log the compilation commands in its log. So to fix this, we need to do two things:
For step 1, we could pass a
I would like to ask for your thoughts for this and it is OK for you, I may kick the contribution for InjectionIII in order for it to work with bazel. Thank you! |
Thanks @xiaohanyu! contributions are always welcome and support for Bazel is something a few people have asked about. It looks like you'd need to make some very delicate changes to an important piece of code (written in Perl) that greps out the logs in SwiftEval.swift. If you want to work on InjectionIII, I recommend switching to the HotReloading version instead which is the same source in a package you add to your project and can iterate over quickly by cloning the project and dragging it into the client project so it takes the place of the configured version. If you find a solution and raise a PR I can look at how dramatic the changes are and asses whether there is a risk of regression for other users of the project. |
Hey @johnno1962, Thanks for the fast reply. Yeah I've already read some code of HotReloading project and I know the combination of shell script and perl thing (and to be honest, also experienced some pain for this, considering whether I can make the contribution to avoid generated shell/perl script and make it pure swift, off topic though, LOL). Would check bazel to see whether bazel has a native method to return the compilation commands. It has
Will surely keep you updated once I have any new discoveries. |
Good Luck! I'm afraid I wouldn't accept a PR that moved away from the Perl code (Sorry!) to Swift as it would likely be much slower and there is tricky code in there to support >256 files in a project that might be difficult to replicate. This is a part of the code that seems to be performing well and I'd be very reluctant to revisit it. Apart from that, see how you go. Perhaps I (or you) could release a separate branch of the project supporting Bazel till it is proven. |
Hey @xiaohanyu, did you make any headway looking at Bazel support? Seems like the very largest projects in the community are using it so it would be nice to be able to offer a solution. Sorry I wasn't very receptive to your offer to rewrite the Perl log searcher but it is something that is currently working quite well. I had a look at Bazel and while I found the small Objective-C example project but I came up against various errors trying follow the QuickStart. Is there any chance you would have the time to prepare a small example Swift project I could have a look at and see if we can move this forward? I imagine the log search could be replaced completely by the Bazel command you mention to extract the recompilation command for a source if I could just get to a working starting point! |
Hey @keith, you seem to have be pretty involved in using Bazel for large Swift iOS projects, can you think of a |
Hi @johnno1962 , I think I could help you with this small project with bazel. There some examples at this repo https://github.com/bazelbuild/rules_apple/tree/master/examples/ios. I'm looking forward to this integration. Let me know if this samples is enough to make some tests. |
Thanks for stepping up @saragiotto, I've been working on a solution for bazel which already works and should be ready in a couple of days. One thing that did crop up which perhaps you could help me with? For injection to work properly the app needs to be linked with "Other Linker Flags" |
I think the ideal solution for this would be to fix grailbio/bazel-compilation-database#71 or adapt https://github.com/hedronvision/bazel-compile-commands-extractor to support generating compile commands from the build. Without one of those solutions, you can pretty easily get the flags for swift targets by doing something like One gotcha you might hit with this, and why I used |
There are a few ways you can do this, in general I think it's common for users to have a custom debug configuration setup in their # some target...
linkopts = select({
"//:dbg": ["-Wl,-interposable"],
"//conditions:default": [],
}), As an aside we have SwiftUI previews working with our bazel project which uses similar infra, so doing something like this is definitely feasible. |
Thanks for the tip! I'm still at sea with bazel -- would it be possible to tell me how/where to apply your suggestion to this BUILD file?
|
In an isolated BUILD file like this, using the linkopts approach I posted above is probably easiest for testing. You can add that directly on the config_setting(
name = "debug_build",
values = {
"compilation_mode": "dbg",
},
) Then in the select you can use |
Thanks, I seem to have found a way adding |
OK, so it was working after all. It was reporting it was failing because there was in fact nothing to interpose. More testing to come from here to see if there are any other rough edges and we'll see if I can post a new release candidate tomorrow some time. |
I've made available a preliminary release that should go a long way towards supporting injection in a project that is using the Bazel build system: https://github.com/johnno1962/InjectionIII/releases/tag/4.5.0RC1 It assumes the source you are injecting is under a directory containing a WORKSPACE file and makes a small patch to https://github.com/bazelbuild/rules_swift to create a link from the parameters files passed by bazel to swiftc to compile a module to a file named after that module in /tmp i.s. /tmp/bazel_<ModuleName>.resp. When you inject a file under the directory contains a WORKSPACE file it scans these files looking for how to recompile the modified source and calls that command, links the object file into a dylib and injects it in the way InjectionIII has worked up to now. To use download the pre-prelease I mention and add the following to.run when your application starts: Bundle(path: "/Applications/InjectionIII.app/Contents/Resources/iOSInjection.bundle")!.load() It is no longer necessary to run the app itself and this fallback to "standalone" mode will watch for source file changes anywhere in your home directory. In addition, for injection to work fully you need to arrange for the options If anybody would like to try it out, let me know how you get on 👍 |
Hi @johnno1962, thank you for taking time and looking into this. I try to take it for spin in our project but unfortunately couldn't make it work. Here are the steps that I do:
We already compile in "dbg" mode in our mini app so I don't need to add it. Here's what printed in console after changing the code:
I assume I need to patch our |
HI @adityadaniel, thanks for taking it for a spin. You didn't see a message
|
Yes, I did see this message
and after restarting the app, Injection seems connected to the app because I see this in console:
However, when I try to change a file and save it, nothing happens. Is this expected? Here's the output of
If I understand correctly, you patch bazel so it output |
Seems like the patching is working. Did you modify a Swift source? So far I've only been using a swift_library entry in the BUILD file. If swiftc has been called, it should have created the /tmp/*.resp link. Did you edit the source file i.e actually change its contents and save after the patching and before the restart? Perhaps this is needed to force the recompile which should create the link. |
For the curious I've pushed the changes required to the HotReloading repo https://github.com/johnno1962/HotReloading/pull/63/files. |
It seems in RC1, you could get into a state where the links in /tmp are not created and no amount of resaving and rebuilding would create them. I've uploaded a new release with a minor change that should make this less likely if someone wants to try it out. https://github.com/johnno1962/InjectionIII/releases/tag/4.5.0RC2 The new version will need to re-patch the bazel worker code to take effect. To force this type: |
Thank you @johnno1962 so much in getting deep in this matter. We are looking forward this support. |
I definitely think a solution using aquery or something else would be preferred to hacking the code in rules_swift, which is pretty fragile and also invalidates the bazel cache, since it changes the inputs to all swift compilation actions. Note that params files are written to disk in bazel-out/, not the absolutely final ones, but you could replicate the argument manipulation if needed |
Re: the -Xlinker -interposable I'm no expert but I think you'd need to configure the bazel project you're using, either where bazel is invoked with
|
I did recently change Xcode versions but
Which is what I'm using (ie opening Xcode from Applications takes me back to the Xcode application I previously opened the sample project with) |
This is what I have in my BUILD file as well. |
The module cache path problem I'll have to see if I can replicate this end. Will reclone and try again from scratch. |
No luck replicating this end but just before I try another build expanding out a link is the |
Yes it is
|
OK, I've replicated your problem by trying to fix it which should mean I can find what the problems is. Where you building the app from inside Xcode using the project generated with |
The former. I was building the app from inside Xcode using the project generated with |
I get the same error after |
It's a strange problem, that worked for me once when I got the error. Anyway, I can replicate now so I'll be able to get to the bottom of it eventually. Stay tuned. |
Thank you so much, I will try doing it from scratch too. |
I've re-uploaded the binary for 4.5.1RC1 with a build number of 7577 if you'd like to give it a try. I couldn't find anything systematic about when it chose one module cache path or the other for compiling the PCH so, for now it tries both! Let me know if this moves us a step long. For the SimpleBazel app to show the colour updating on injection you'll need to add the following method in ViewController.swift:
|
It works! 🎉 |
Tried on a more involved project and got the following, looks like a permission issue:
|
Hurrah! The new error is probably to do with sandboxing bazel seems to use. Are you able to send me (GitHub at johnholdsworth.com if need be) the /tmp/bazel_...params file it was using? I probably just need to filter out an -emit-something-or-other option to swiftc. |
|
Thanks, looks like the -emit-objc-header-path needs to be filtered out, I've uploaded the 4.5.1RC1 release again, build 7578. Can you try it please? |
I get this now:
|
Seems to be a regex run berzerk. Could you send me the /tmp/bazel_ArchetypeWelcomeImpl.params and /tmp/bazel_ArchetypeWelcomeImpl.params.copy files or rather the command.sh file please. |
|
|
Thanks for getting back so quickly. I've pushed another 4.5.1RC1 release, build 7579. I needed to tighten up a regex to exclude newlines in the code that detects the sdk path to use during linking. |
Had to revise the load bearing regex again, build number now 7580. |
It worked 🙌 thank you for iterating so quickly, this is amazing! |
Great! Let me know if you come up against something else but we're really moving things forward now. Thanks! |
I'll close this again as I'm not getting any more feedback at the moment. |
Has anyone gotten this working with bazel from the command line (i.e. w/o Tulsi nor rules_xcodeproj)? I was able to try the first method outlined in https://github.com/johnno1962/InjectionIII/blob/main/BAZEL.md using the macos_application(
name = "macos",
linkopts = ["-interposable"],
) and run the InjectionIII app, but then get:
with the 3rd line appearing once I try modifying
It seems to not find /path/to/my/workspace. |
Hi support for BAZEl is pretty limited and it need you to build inside Xcode so it can find the logs and parse out the commands to build files. I don't think I'll be able to support this configuration, sorry. |
I try to use Injection in our project which is quite large and build using Bazel. When rebuilding, I got this message in console
How can I set up custom derived data path on Injection?
The text was updated successfully, but these errors were encountered: