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

Adds a file to hold a map from class/method to headerfile #58054

Merged
merged 14 commits into from
Jul 16, 2024

Conversation

3nids
Copy link
Member

@3nids 3nids commented Jul 10, 2024

This file will allow us to have direct links from PyQGIS API docs to cpp header to allow direct editing.

This partially tackles qgis/pyqgis-api-docs-builder#37

Screen.Recording.2024-07-10.at.12.08.02.mov

@nyalldawson
Copy link
Collaborator

Pretty cool! 🤩

@lbartoletti
Copy link
Member

Woo, many thanks! Is it possible to also have a link to the C++ API page?

@github-actions github-actions bot added this to the 3.40.0 milestone Jul 10, 2024
@3nids
Copy link
Member Author

3nids commented Jul 10, 2024

Woo, many thanks! Is it possible to also have a link to the C++ API page?

Not sure, I used an extension to show the link and it supports only one link.

@3nids
Copy link
Member Author

3nids commented Jul 10, 2024

unrelated test failure
this is ready on my side :)

@troopa81
Copy link
Contributor

I like the proposal. One thing that bother me is that the class map YAML files would be modified every time someone modifies the API. We have already the Qt5 and 6 sip files and all these generated files are not helping review.

Could we just generate those files when needed ? meaning when documentation is generated?

@3nids
Copy link
Member Author

3nids commented Jul 10, 2024

@troopa81 indeed it might be a bit too much traffic. Just thinking that since we have the headers installed this would be possible. I will give it a try.

Or you rewrite sipify with clang-parser to we have this at build step only? 😁

@3nids
Copy link
Member Author

3nids commented Jul 10, 2024

This is not that trivial in the end

  • I need to install the dev packages to get the headers which is a bit painful as I need a specific version
  • I need to pull sipify.pl + sipify_all.sh
  • We need to install sipify.yaml config (no big deal)

Possible solutions:

  • also publish images with te dev package
  • fetch the QGIS source code to get sipify + headers

I'm not super happy with both approaches.

One thing is that we could easily drop the Qt6 map file, that would lower the noise by 2. Would this be acceptable (done in the following commit)?

@troopa81
Copy link
Contributor

@3nids Not sure to understand how exactly the process of building the documentation work but could we not generate the yaml files when we build the python binding instead of just installing them ?

@3nids
Copy link
Member Author

3nids commented Jul 11, 2024

could we not generate the yaml files when we build the python binding

So calling sipify_all from cmake?
This would add a significant time overhead when building the bindings. I'm not sure that's acceptable.

I'm trying to avoid extra complexity in the building process of the Python docs (I would like to avoid maintaining a new workflow to actually compile QGIS to build the API docs).

When you add a method to a header, then you get the same number of modified lines in the map file than the number of following non-private methods in the header (due to the modification of line number for the methods). Let's say we talk about ~5-10 lines changes.

@3nids
Copy link
Member Author

3nids commented Jul 11, 2024

Another approach would be to run this on master branch only and automatically so we don't get any noise in the PRs. This would auto-commit on master and release branches. Basically the same workflow than the one I just merged here

@troopa81
Copy link
Contributor

So calling sipify_all from cmake?
This would add a significant time overhead when building the bindings. I'm not sure that's acceptable.

Is it that slow if we run sipify_all.sh to generate only those map files ? maybe only when installing ?

Another approach would be to run this on master branch only and automatically so we don't get any noise in the PRs. This would auto-commit on master and release branches. Basically the same workflow than the one I just merged #58057

I would have prefer not to have another set of generated files in git but it might be a good middle ground regarding our actual doc workflow. Could it be done just after merge without mentioning anything in the PR?

@3nids
Copy link
Member Author

3nids commented Jul 12, 2024

I would have prefer not to have another set of generated files in git but it might be a good middle ground regarding our actual doc workflow. Could it be done just after merge without mentioning anything in the PR?

Yes it would be done in the branches only, so no traffic/noise in PRs.
I have adapted the sipify workflow in 3767c99

Copy link

github-actions bot commented Jul 12, 2024

🪟 Windows builds ready!

Windows builds of this PR are available for testing here. Debug symbols for this build are available here.

(Built from commit 4bf2e17)

Copy link
Contributor

@troopa81 troopa81 left a comment

Choose a reason for hiding this comment

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

Sounds good to me!

@3nids
Copy link
Member Author

3nids commented Jul 16, 2024

@troopa81 thanks. Let's go like this and we can still revisit the approach if we're not happy.

@3nids 3nids merged commit 6e99331 into qgis:master Jul 16, 2024
28 checks passed
@3nids 3nids deleted the sip-class-map branch July 16, 2024 09:40
@qgis-bot
Copy link
Collaborator

The backport to release-3_34 failed:

The process '/usr/bin/git' failed with exit code 1
stderr
error: could not apply 02a6e27847d... create map file for class / header files for PyQGIS API docs
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".
hint: Disable this message with "git config advice.mergeConflict false"

stdout
Auto-merging scripts/sipify.pl
CONFLICT (content): Merge conflict in scripts/sipify.pl
Auto-merging scripts/sipify_all.sh
CONFLICT (content): Merge conflict in scripts/sipify_all.sh
Auto-merging tests/code_layout/sipify/test_sipfiles.sh
CONFLICT (content): Merge conflict in tests/code_layout/sipify/test_sipfiles.sh

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-release-3_34 release-3_34
# Navigate to the new working tree
cd .worktrees/backport-release-3_34
# Create a new branch
git switch --create backport-58054-to-release-3_34
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick 02a6e27847d5628ab59a34da9b87d176ed02734d,9df54cca22b443af3e4dfe0248c82808768792b8,4f51d2e1f40eaa926130eed86cb8b4f6d5703ac2,7d2b4bb74e04f86904660f88ff0c4d9f25a3170d,dce64ab8601a4b4690be63005e78c08b513f3bfd,9f4b910d2e3b0226a16db0df4612d7c2f8cb2d3c,7b92924954236cb8555ebca9ee615cb7cc4a1e0a,751745ecada8b6d0230903290b3a1c9b0c8b5a44,46a18c1389319cd30ea8151779ed236b2f81849e,f1dd3eb8e6c78326967248438e42404045ecf623,78658fa9c10853dc6dcb9167cb0d0a6d1a29606d,f1c09ad3593b75be114b4bb4f6a4d66d3d0ed5ce,3767c9961523777b53ddf40bebbc5d2ee1ff9b1d,4bf2e17f3ff5b5b31f026bd0ae13b913832769b0
# Push it to GitHub
git push --set-upstream origin backport-58054-to-release-3_34
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-release-3_34

Then, create a pull request where the base branch is release-3_34 and the compare/head branch is backport-58054-to-release-3_34.

@qgis-bot qgis-bot added the failed backport The automated backport attempt failed, needs a manual backport label Jul 16, 2024
@qgis-bot
Copy link
Collaborator

The backport to release-3_38 failed:

The process '/usr/bin/git' failed with exit code 1
stderr
error: could not apply 3767c996152... do not test class_map files + auto sipify_all on branches
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".
hint: Disable this message with "git config advice.mergeConflict false"

stdout
[backport-58054-to-release-3_38 36a81923354] create map file for class / header files for PyQGIS API docs
 Author: Denis Rouzaud <denis.rouzaud@gmail.com>
 Date: Wed Jul 10 09:45:29 2024 +0200
 3 files changed, 22 insertions(+), 5 deletions(-)
[backport-58054-to-release-3_38 5c94e094380] create map files
 Author: Denis Rouzaud <denis.rouzaud@gmail.com>
 Date: Wed Jul 10 09:47:09 2024 +0200
 10 files changed, 4157 insertions(+)
 create mode 100644 python/3d/class_map.yaml
 create mode 100644 python/PyQt6/3d/class_map.yaml
 create mode 100644 python/PyQt6/analysis/class_map.yaml
 create mode 100644 python/PyQt6/core/class_map.yaml
 create mode 100644 python/PyQt6/gui/class_map.yaml
 create mode 100644 python/PyQt6/server/class_map.yaml
 create mode 100644 python/analysis/class_map.yaml
 create mode 100644 python/core/class_map.yaml
 create mode 100644 python/gui/class_map.yaml
 create mode 100644 python/server/class_map.yaml
[backport-58054-to-release-3_38 bce5b861926] install file
 Author: Denis Rouzaud <denis.rouzaud@gmail.com>
 Date: Wed Jul 10 09:52:11 2024 +0200
 1 file changed, 3 insertions(+)
[backport-58054-to-release-3_38 9f17391a9e8] add line numbers
 Author: Denis Rouzaud <denis.rouzaud@gmail.com>
 Date: Wed Jul 10 10:40:26 2024 +0200
 11 files changed, 4158 insertions(+), 4158 deletions(-)
[backport-58054-to-release-3_38 d28769fabd5] also add methods
 Author: Denis Rouzaud <denis.rouzaud@gmail.com>
 Date: Wed Jul 10 11:45:49 2024 +0200
 11 files changed, 28592 insertions(+), 3 deletions(-)
[backport-58054-to-release-3_38 d83b707eb65] wait to sort
 Author: Denis Rouzaud <denis.rouzaud@gmail.com>
 Date: Wed Jul 10 12:23:10 2024 +0200
 11 files changed, 14823 insertions(+), 14801 deletions(-)
[backport-58054-to-release-3_38 2e95989a53e] more methods
 Author: Denis Rouzaud <denis.rouzaud@gmail.com>
 Date: Wed Jul 10 14:06:40 2024 +0200
 11 files changed, 23569 insertions(+), 1173 deletions(-)
[backport-58054-to-release-3_38 47fd80f7d30] fix CMakeLists install
 Author: Denis Rouzaud <denis.rouzaud@gmail.com>
 Date: Wed Jul 10 14:25:03 2024 +0200
 1 file changed, 1 insertion(+), 1 deletion(-)
[backport-58054-to-release-3_38 544f8d6a406] fix sorting
 Author: Denis Rouzaud <denis.rouzaud@gmail.com>
 Date: Wed Jul 10 14:39:02 2024 +0200
 12 files changed, 26503 insertions(+), 26436 deletions(-)
[backport-58054-to-release-3_38 975e7aed127] fix spell check
 Author: Denis Rouzaud <denis.rouzaud@gmail.com>
 Date: Wed Jul 10 15:01:55 2024 +0200
 1 file changed, 1 insertion(+), 1 deletion(-)
[backport-58054-to-release-3_38 a25e9ac011f] remove map files for Qt6
 Author: Denis Rouzaud <denis.rouzaud@gmail.com>
 Date: Wed Jul 10 20:02:09 2024 +0200
 6 files changed, 15 insertions(+), 27615 deletions(-)
 delete mode 100644 python/PyQt6/3d/class_map.yaml
 delete mode 100644 python/PyQt6/analysis/class_map.yaml
 delete mode 100644 python/PyQt6/core/class_map.yaml
 delete mode 100644 python/PyQt6/gui/class_map.yaml
 delete mode 100644 python/PyQt6/server/class_map.yaml
[backport-58054-to-release-3_38 7b4331ed083] Revert "remove map files for Qt6"
 Author: Denis Rouzaud <denis.rouzaud@gmail.com>
 Date: Thu Jul 11 11:59:36 2024 +0200
 6 files changed, 27615 insertions(+), 15 deletions(-)
 create mode 100644 python/PyQt6/3d/class_map.yaml
 create mode 100644 python/PyQt6/analysis/class_map.yaml
 create mode 100644 python/PyQt6/core/class_map.yaml
 create mode 100644 python/PyQt6/gui/class_map.yaml
 create mode 100644 python/PyQt6/server/class_map.yaml
CONFLICT (rename/delete): .github/workflows/pr-fixer-bot.yml renamed to .github/workflows/sipify-bot.yml in 3767c996152 (do not test class_map files + auto sipify_all on branches), but deleted in HEAD.
CONFLICT (modify/delete): .github/workflows/sipify-bot.yml deleted in HEAD and modified in 3767c996152 (do not test class_map files + auto sipify_all on branches).  Version 3767c996152 (do not test class_map files + auto sipify_all on branches) of .github/workflows/sipify-bot.yml left in tree.

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-release-3_38 release-3_38
# Navigate to the new working tree
cd .worktrees/backport-release-3_38
# Create a new branch
git switch --create backport-58054-to-release-3_38
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick 02a6e27847d5628ab59a34da9b87d176ed02734d,9df54cca22b443af3e4dfe0248c82808768792b8,4f51d2e1f40eaa926130eed86cb8b4f6d5703ac2,7d2b4bb74e04f86904660f88ff0c4d9f25a3170d,dce64ab8601a4b4690be63005e78c08b513f3bfd,9f4b910d2e3b0226a16db0df4612d7c2f8cb2d3c,7b92924954236cb8555ebca9ee615cb7cc4a1e0a,751745ecada8b6d0230903290b3a1c9b0c8b5a44,46a18c1389319cd30ea8151779ed236b2f81849e,f1dd3eb8e6c78326967248438e42404045ecf623,78658fa9c10853dc6dcb9167cb0d0a6d1a29606d,f1c09ad3593b75be114b4bb4f6a4d66d3d0ed5ce,3767c9961523777b53ddf40bebbc5d2ee1ff9b1d,4bf2e17f3ff5b5b31f026bd0ae13b913832769b0
# Push it to GitHub
git push --set-upstream origin backport-58054-to-release-3_38
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-release-3_38

Then, create a pull request where the base branch is release-3_38 and the compare/head branch is backport-58054-to-release-3_38.

3nids added a commit to 3nids/QGIS that referenced this pull request Jul 16, 2024
* create map file for class / header files for PyQGIS API docs

* create map files

* install file

* add line numbers

* also add methods

* wait to sort

* more methods

* fix CMakeLists install

* fix sorting

* fix spell check

* remove map files for Qt6

* Revert "remove map files for Qt6"

This reverts commit 972f483.

* do not test class_map files + auto sipify_all on branches

* fix warning
3nids added a commit to 3nids/QGIS that referenced this pull request Jul 16, 2024
* create map file for class / header files for PyQGIS API docs

* create map files

* install file

* add line numbers

* also add methods

* wait to sort

* more methods

* fix CMakeLists install

* fix sorting

* fix spell check

* remove map files for Qt6

* Revert "remove map files for Qt6"

This reverts commit 972f483.

* do not test class_map files + auto sipify_all on branches

* fix warning
3nids added a commit that referenced this pull request Jul 16, 2024
* Adds a file to hold a map from class/method to headerfile (#58054)

* create map file for class / header files for PyQGIS API docs

* create map files

* install file

* add line numbers

* also add methods

* wait to sort

* more methods

* fix CMakeLists install

* fix sorting

* fix spell check

* remove map files for Qt6

* Revert "remove map files for Qt6"

This reverts commit 972f483.

* do not test class_map files + auto sipify_all on branches

* fix warning

* fix sipify bot

* fix warning

* no error if nothing to commit
3nids added a commit that referenced this pull request Jul 16, 2024
* Adds a file to hold a map from class/method to headerfile (#58054)

* create map file for class / header files for PyQGIS API docs

* create map files

* install file

* add line numbers

* also add methods

* wait to sort

* more methods

* fix CMakeLists install

* fix sorting

* fix spell check

* remove map files for Qt6

* Revert "remove map files for Qt6"

This reverts commit 972f483.

* do not test class_map files + auto sipify_all on branches

* fix warning

* fix sipify bot

* no error if nothing to commit
@3nids 3nids mentioned this pull request Jul 22, 2024
@kannes
Copy link
Contributor

kannes commented Jul 26, 2024

This is really useful, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport release-3_34 backport release-3_38 failed backport The automated backport attempt failed, needs a manual backport
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants