Skip to content
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

Ide instructions doc #772 #774

Merged

Conversation

avrecko
Copy link
Contributor

@avrecko avrecko commented May 24, 2020

Feel free to change the text as much as you like. I'd just like to see IDE support mentioned.

Alternatively the CMakeLists.txt generation could be mentioned.

@avrecko avrecko force-pushed the issue_772_ide_instructions_doc branch from 9d7f747 to 21328fd Compare May 24, 2020 10:26
@avrecko avrecko force-pushed the issue_772_ide_instructions_doc branch from 450771a to 816c06c Compare May 24, 2020 11:00
No specific instructions are available.

You might find Gyp generators helpful. Output is not guaranteed to work.
Manual editing might be necessary.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think if it will be useful to mention what manual editing you have done to make it work in CLion?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. I'll think about the docs some more. Maybe better for me to just focus on CLion. I'll revisit this over the weekend.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@avrecko any progress in this topic?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zdanek I am preoccupied with regular work. I don't see myself taking the time to revisit this. Maybe you can take a look or @kqyang can either approve or decline this pull request?

I didn't get it to build with the IDE just enough for the code completion to work if I remember correctly. I just ended up removing a bunch of stuff from CMakeList. Hardly good engineering. Not the quality standard I'd target for. Best if a professional C developer makes the instructions.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I use VIM with youcompleteme personally, which just works without the need to do extra configuration IIRC. It has auto completion too.

And sure, I'll merge it as is. I think it is already useful for some people in the current state.

@zdanek, if you have anything to add, feel free to send a PR to update it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I managed to create CMakeLists.txt and CLion accepted it.
I was able to edit code with autocompletition. But it required removing about dozen of cc files from various GYP files as they were absent in source tree. I think it was some kind of mess that arose through the years of coding.
I will try to repeat those steps and check if it does not fail any of the builds / build targets and eventually make an MR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That will be great. Thanks.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kqyang I have fixed gyp files and packager/tools/gyp/pylib/gyp/generator/cmake.py and now it creates proper CMakeLists.txt that can be used with build tool or IDE based on cmake. CLion is a great example.

Problem is with cmake.py. It's from https://chromium.googlesource.com/external/gyp I'm not very skilled at cmake. Maybe it can be fixed differently. I will post on GYP tools mailing list regarding this.

There's third concern. nocompile.gni refers to nocompile_driver.py script which is not present in source tree.
If not provided, cmake will fail. Eventually it can be created as empty file, because it's not invoked in normal compilation process.

There are some GYP files that should be changed because they point to non existing files in source tree but the GYP files are from other git modules, for example https://chromium.googlesource.com/chromium/src/build
but still the repo moved forward and the GYP file no longer exists in newer source tree.

This is a bit complicated.
It requires to upgrade submodules to versions where GYP files are fixed but not so new as Shaka will not compile with latest versions of submodules.

What to do?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zdanek, Great to know that you have made it working! Thanks for looking into it.

Problem is with cmake.py. It's from https://chromium.googlesource.com/external/gyp I'm not very skilled at cmake. Maybe it can be fixed differently. I will post on GYP tools mailing list regarding this.

You'll need to submit a PR to chromium for cmake changes: https://chromium.googlesource.com/chromium/src/+/master/docs/contributing.md.

nocompile.gni refers to nocompile_driver.py script

We do not use GN right now, so it shouldn't be invoked. Do you know which script invokes nocompile.gni?

There are some GYP files that should be changed

Do you mean that in order to make cmake working, the GYP files need to be changed too? What are the change OOC?

And yes, the chromium upstream has deprecated GYP, so we won't be able to change the GYP files. Can it be workarounded?

@kqyang kqyang merged commit 449945d into shaka-project:master Feb 3, 2021
@github-actions github-actions bot added the status: archived Archived and locked; will not be updated label Jul 24, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: archived Archived and locked; will not be updated
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants