-
Notifications
You must be signed in to change notification settings - Fork 1
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
CPL-7730: Fission support #73
Merged
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
spirsch
reviewed
Feb 2, 2023
wmann-celonis
approved these changes
Feb 16, 2023
spirsch
reviewed
Feb 16, 2023
spirsch
approved these changes
Feb 17, 2023
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.
Adds fission support for homcc:
-gsplit-dwarf
flagdwo
file (for each object file) from the server to the client using theCompilationResultMessage
-o
), and previously we were ignoring the output argument on the server side, we needed to include output argument handling on the server. Also, output paths must be relative so that the path inside the object files is also relative and not absolute (which would break on the client then). The server now also directly returns the path for the client to save the object file, the special handling inside the client has been removed.E2e tests and a unit tests for utilities are included.
Compatibility should be theoretically ensured for both: older clients connecting to newer servers and the other way around as we just added a field. There could be issues with compatibility regarding the changed output path behavior, I have to think about that again. So before we release, we should test compatibility.
Below are some findings on the initial implementation that prevented it from working, I keep it here as it may be useful at a later point in time:
Steps to reproduce:
GDB error in case of using this with homcc
Inspecting the object files with
readelf
yields the following:Note for the object file generated by
homcc
, the name stored inDW_AT_dwo_name
is not correct. This is because we compile the cpp file on the server side without the output flag, and therefore the .dwo file has a different name it refers to.The only possible solution that comes to my mind is to move to
-o
logic to the server, and also use-o
on the server. The client then just saves the .o/.dwo file under the path returned from the server and does not need to have any-o
file logic any more.