mkhtml: Generate local JSON file with core modules and their last commit #2140

tmszi · 2022-01-29T14:06:40Z

In the context of creating and using a core_modules_with_last_commit.json file after creating and submitting a new release tag to remote repository e.g. git tag -a 8.2.0 -m "Release 8.2.0" and git push origin 8.2.0 a new draft is automatically created with GitHub action workflow "Create new release draft" #2224 with attached two files core_modules_with_last_commit.json and core_modules_with_last_commit.patch.

JSON file data structure:

"r.pack": {
    "commit": "547ff44e6aecfb4c9cbf6a4717fc14e521bec0be",
    "date": "2022-02-03T11:10:06+01:00"
},

During compilation source code without .git dir, this local file "core_modules_with_last_commit.json" is used instead of getting Git commits from remote GitHub API server.

wenzeslaus

I need to look at this more, but it seems that something like this is the only way to get all the info from Git. I assume others already looked into bending Git to be more like Subversion and that didn't work. I would still ask though, why is carrying this generated file in the code necessary?

As for the implementation, the automatic commit won't work well here as far as I can tell. Doing on the PR branch will record wrong commit hashes. Doing it on the main branch will result in doubling the amount of commits as every commit (merge) will require another commit as an update for this file. (And also disabling some of the mandatory checks may be needed to make it work.)

I came up with two scenarios which could work:

Make the workflow a dispatch workflow (or whatever these are called) so that it is called manually. Part of the release procedure would be to trigger this workflow, it would create a PR against the release branch (or commit, but PR sounds safer!). This is closer to your current proposal, than the other option.
Make the generated file a GitHub Actions artifact (possible for all commits on all branches) and let people download it from GitHub when needed or, for convenience, upload it somewhere from GitHub actions (using scp or FTP). This could be part of creating the release tarball. Requires more manual work, but doesn't require to have the generated file in the source code.

tmszi · 2022-01-30T06:37:39Z

As for the implementation, the automatic commit won't work well here as far as I can tell. Doing on the PR branch will record wrong commit hashes. Doing it on the main branch will result in doubling the amount of commits as every commit (merge) will require another commit as an update for this file. (And also disabling some of the mandatory checks may be needed to make it work.)

Auto commit has some limitation but in our case Git repository allow Squash and Merge and if this GitHub action is allowed on the only main branch, it is work as expected (update this JSON file is triggered on push to main branch and one auto commit is pushed). I tested it on my test repository and it worked as expected.

lucadelu · 2022-02-11T09:35:55Z

Tested with 7.8.7 release branch and it work properly

--------------------------------------------------
Started compilation: Fri Feb 11 09:20:13 UTC 2022
--
Errors in:
No errors detected.
--
Finished compilation: Fri Feb 11 09:29:52 UTC 2022

tmszi · 2022-02-12T04:35:41Z

I need to look at this more, but it seems that something like this is the only way to get all the info from Git. I assume others already looked into bending Git to be more like Subversion and that didn't work. I would still ask though, why is carrying this generated file in the code necessary?

Yes you are right it is not necessary.

Make the generated file a GitHub Actions artifact (possible for all commits on all branches) and let people download it from GitHub when needed or, for convenience, upload it somewhere from GitHub actions (using scp or FTP). This could be part of creating the release tarball. Requires more manual work, but doesn't require to have the generated file in the source code.

I decided to implement this procedure. The resulting GitHub workflow pgms-with-last-commit-patch-file artifact will create a pgms_with_last_commit.patch (new pgms_with_last_commit.json file) that will be downloadable as zip file for the GRASS GIS GNU/Linux distribution maintainer and applicable as a patch in its package build process.

tmszi · 2022-02-13T10:01:26Z

@neteler, I think we should include this PR in the next release 8.0.1 and 7.8.7 and solve GitHub REST API rate limit completely (which means every core module manual page will have commit and commit date, when GRASS GIS is compile from release archive which doesn't have Git).

The generated pgms_with_last_commit.json file can then be downloaded from the GitHub action workflow pgms-with-last-commit-patch-file artifact (for branch) and pasted into the release archive during creation.

…mmit For every pushed commit GitHub Generate PGMS last commit JSON file action worflow update "pgms_with_last_commit.json" file (auto commit), which contains all PGMS (core modules) with last commit. JSON file data structure: {"pgm": {"commit": "git log -1 PGM src dir"}, ...} During compilation source code without .git dir, this local file "pgms_with_last_commit.json" is used instead of getting Git commit from remote GitHub API server.

This reverts commit 29a7e7c.

… last commit" This reverts commit a3f56c0.

…mmit For every pushed commit GitHub 'Generate PGMS last commit JSON file action worflow artifact' create "pgms_with_last_commit.patch" patch file, which contains all PGMS (core modules) with last commit. Which is donwloadable from GitHub actions workflows page as pgms-with-last-commit-patch-file.zip file. JSON file data structure: {"pgm": {"commit": "git log -1 PGM src dir"}, ...} During compilation source code without .git dir, this local file "pgms_with_last_commit.json" is used instead of getting Git commit from remote GitHub API server.

neteler · 2022-02-13T12:55:37Z

I think we should include this PR in the next release 8.0.1 and 7.8.7 and solve GitHub REST API rate limit completely

Thanks for your work on this PR! I am a bit undecided if 8.0.1 and 7.8.7 or better 8.0.2 and 7.8.8? If the former, then we will need a longer test phase (I just published 8.0.1RC1 yesterday, see grass-dev mailing list). I suggest to discuss it on grass-dev to reach more people.

…mmit For every pushed commit GitHub Generate PGMs last commit JSON file action worflow generate "pgms_with_last_commit.json" file (pgms-with-last-commit-patch-file artifact that will be downloadable as zip file), which contains all PGMs (core modules) with last commit. JSON file data structure: {"PGM": {"commit": "git log -1 PGM src dir"}, ...} During compilation source code without .git dir, this local file "pgms_with_last_commit.json" is used instead of getting Git commit from remote GitHub API REST server.

lucadelu · 2022-02-13T22:37:51Z

I would prefer 7.8.7 and 8.0.1

wenzeslaus · 2022-02-14T03:58:37Z

I prefer after 7.8.7 and 8.0.1 or later. This is not even reviewed, while 8.0.1 is ready to go.

Let's not backport large or invasive changes and let's not backport quickly or right before a release. As far as I'm concerned, this can sit on the main branch all the way to 8.2.0. Date in a man page is not important enough to risk breaking compilation on a release branch.

tmszi · 2022-02-14T05:56:15Z

Date in a man page is not important enough to risk breaking compilation on a release branch.

I will try to explain in more detail what other consequences the exceeded GitHub REST API rate limit has during compilation, which is reported in the issue #2186, and that is why I suggest integrating it into 8.0.1 and 7.8.7 as soon as possible after thorough testing:

Compilation GRASS GIS from source code without Git was successfull on computer which never has installed GRASS GIS before (.grass8 dir doesn't exists)
GitHub REST API rate limit was exceeded during compilation
After GRASS GIS installation you want install some new addon: g.extension i.sentinel
You get error message GitHub REST API rate limit was exceeded, and user must wait (1 hour). Information when rate limit is freed is not included, and this is reason why I created PR scripts/g.extension: make GitHub REST API server rate limit error message more verbose #2193

GRASS nc_basic_spm_grass7/PERMANENT:~ > g.extension i.sentinel
ERROR: The download of the JSON file with add-ons paths from the GitHub
       REST API server wasn't successful, rate limit exceeded, 60 requests
       per hour per your computer IP address. The previous downloaded JSON
       file will not be used, because does not exists. You can try it again
       on Saturday Feb 12 14:57:22 2022.

Non technical user don't understand what is GitHub REST API rate limit without better explanation.

As already mentioned, the main problem is that we are depend on proprietary GitHub REST API (g.extension module) which has limitation and who has control over us, not we over him.

Let's not backport large or invasive changes and let's not backport quickly or right before a release.

It is just a sugestion that explains in much more detail why it will be good to implement these changes as soon as possible.

Code changes with this PR:

New GitHub action workflow artifact, which I tested separately on my test repository
Simple new script which generate new JSON file
Open and read JSON file

Code changes with PR #2193:

Changed error message string

ninsbl · 2022-02-14T09:29:56Z

Please see my comments on #2193
We should make sure that compilation and installation of addons works in 7.8.7 and 8.0.1.
If merging this PR is required for that I would agree to merge. However, it seems that a workaround for the related compilation bug is in place in 7.8.7 and 8.0.1, so from that point of view I agree with @wenzeslaus that we rather test this more broadly in the development version and include the more elegant and accurate handling of dates manuals in 8.2.

.github/workflows/gen_pgms_last_commit_json_file.yml

ninsbl

OK. I invstigated the possibility to deploy to a gist to circumvent the API (content could be downloaded as raw file directly), but only users seem to be able to own gists (not organizations (here osgeo) and repos (grass)...
So, I guess artifacts are the best option if we do not publish the JSON file other places (which would have other drawbacks).
Another question is if we could find a mechanism where the JSON file is attempted to be downloaded only once (i.e. at start of compilation) or if it has been updated since the last download or lder than e.g one hour.

That said, this PR definately improves the current situation.

wenzeslaus

This seems like a step in the right direction if we want to harvest all the information from Git.

If I remember correctly, GitHub artifacts don't have predictable URLs. Any plans for that? We could upload somewhere from GitHub Actions using something like FTP/SCP and secrets.

I would like to see this running on PRs as well for testing purposes, esp., for cases when you make change here. The additional_checks.yml workflow/job is meant for random things like this, so you could add the generator script there with some check of the JSON content.

.github/workflows/gen_pgms_last_commit_json_file.yml

utils/mkhtml.py

wenzeslaus · 2022-02-16T22:41:19Z

utils/mkhtml.py

-                        commit[0]["commit"]["author"]["date"],
-                        "%Y-%m-%dT%H:%M:%SZ",
-                    ).strftime(datetime_format)
+            if gs:


The whole "can't import grass.script therefore I'm compiling" is shaky, but perhaps good enough for this PR.

utils/mkhtml.py

utils/gen_pgms_last_commit.py

.github/workflows/gen_pgms_last_commit_json_file.yml

wenzeslaus · 2022-02-17T03:06:41Z

...As already mentioned, the main problem is that we are depend on proprietary GitHub REST API (g.extension module) which has limitation and who has control over us, not we over him.

Right, I think everyone agrees with that. My point is that there is no reason to have anything like that in the source code in the first place when it brings no benefit and since the code as is doesn't work, we can just drop it and, in some cases, live with some missing metadata in manual pages for 7.8 and 8.0. For 8.2, we can see how this PR works.

tmszi · 2022-02-17T11:46:47Z

If I remember correctly, GitHub artifacts don't have predictable URLs. Any plans for that? We could upload somewhere from GitHub Actions using something like FTP/SCP and secrets.

I assumed that this one manual step would be enough. When we want make new release (source code without Git), person which make this release via GitHub page, first download pgms_with_last_commit.json file from last GitHub workflow pgms-with-last-commit-patch-file artifact to him computer and than upload as attach into process of making GitHub GRASS GIS release. ~~File will be part of archive e.g. https://github.com/OSGeo/grass/archive/refs/tags/8.0.1RC1.tar.gz.~~

wenzeslaus · 2022-02-17T14:16:44Z

....we want make new release (source code without Git)...first download pgms_with_last_commit.json file from last GitHub workflow...than upload as attach into process...

The release procedure is already a lot of work. We should be working towards minimizing the steps and automating ideally everything.

...file will be part of archive...

The GitHub page says you can include binary files...in the binaries box. I don' see anything about combining that with the tarball.

However, the download links in a release are stable and it seems you can upload to release from GitHub actions (softprops/action-gh-release, svenstaro/upload-release-action, JasonEtco/upload-to-release, ...).

wenzeslaus

The release draft action is a great improvement. I'm not sure about the actions/create-release part since it is kind of buried in the context of this PR. The actions/upload-release-asset is more clear, but the this specific action is not maintained (of the ones I linked is a suggested replacement). Would you consider splitting the PR?

I still think something the like python utils/generate_core_modules_with_last_commit_json_file.py . should run also in additional_checks.yaml with some subsequent check (like load to Python and check basic structure or one of the values).

Thanks for the rename. Perhaps the new one is little too long? generate_last_commit_file.py gets most of the important things in.

utils/generate_core_modules_with_last_commit_json_file.py

utils/mkhtml.py

…output param

…capture_output param" This reverts commit 3f61f88. capture_output parmeter is supported by Python version >= 3.8

tmszi · 2022-02-19T18:11:37Z

The release draft action is a great improvement. I'm not sure about the actions/create-release part since it is kind of buried in the context of this PR. The actions/upload-release-asset is more clear, but the this specific action is not maintained (of the ones I linked is a suggested replacement). Would you consider splitting the PR?

Ok. I opened new PR #2224.

I still think something the like python utils/generate_core_modules_with_last_commit_json_file.py . should run also in additional_checks.yaml with some subsequent check (like load to Python and check basic structure or one of the values).

Ok. I created pytest file pytest utils/test_generate_last_commit_file.py and incrorporate into additional_checks.yaml file GitHub workflow action.

Thanks for the rename. Perhaps the new one is little too long? generate_last_commit_file.py gets most of the important things in.

Yes, I agree with you.

I have included all your previous suggestions.

wenzeslaus

Getting there. Awesome example of full pytest usage.

utils/test_generate_last_commit_file.py

utils/mkhtml.py

…rted module

tmszi · 2022-02-20T09:32:02Z

I also integrated the remaining suggestions.

wenzeslaus

I'm not sure about the use of UNIX timestamp instead of a readable date and use of sophisticated pytest with depends on an external state, but I think this can be merged.

I don't know where documentation should go, esp. in context of #2224 (automated upload to release draft).

tmszi · 2022-04-13T11:28:10Z

I'm not sure about the use of UNIX timestamp instead of a readable date and use of sophisticated pytest with depends on an external state, but I think this can be merged.

All right, I'll change to the readable date format ISO 8601.

wenzeslaus · 2022-05-16T21:18:06Z

This is in 8.2 branch and it is not a general backport material (so no 8.0 which has another fix), so removing the label.

… last commit (OSGeo#2140) For every new pushed repository branch Git tag e.g. `git tag -a 8.2.0 -m "Release 8.2.0"` and `git push origin 8.2.0` is automatically with GitHub "Create new release draft" action workflow created new release draft which include downloadable core_modules_with_last_commit.json and core_modules_with_last_commit.patch file. During compilation source code without .git dir, this local file is used instead of getting Git commit from remote GitHub API REST server. JSON file data structure: "r.pack": { "commit": "547ff44e6aecfb4c9cbf6a4717fc14e521bec0be", "date": "2022-02-03T11:10:06+01:00" }, commit key value is commit hash date key value is author date (strict ISO 8601 date time format)

Backport of #2550 to G78, fixing Python 3.11 error after deprecation. * fix #2538 * Use getencoding() where only encoding is needed; getlocale() otherwise Additional changes: - sync of `tools/mkhtml.py` with `main` branch - selected backport from #2140 to get `utils/generate_last_commit_file.py` - selective sync of `lib/init/grass.py`

… last commit (OSGeo#2140) For every new pushed repository branch Git tag e.g. `git tag -a 8.2.0 -m "Release 8.2.0"` and `git push origin 8.2.0` is automatically with GitHub "Create new release draft" action workflow created new release draft which include downloadable core_modules_with_last_commit.json and core_modules_with_last_commit.patch file. During compilation source code without .git dir, this local file is used instead of getting Git commit from remote GitHub API REST server. JSON file data structure: "r.pack": { "commit": "547ff44e6aecfb4c9cbf6a4717fc14e521bec0be", "date": "2022-02-03T11:10:06+01:00" }, commit key value is commit hash date key value is author date (strict ISO 8601 date time format)

tmszi added the bug Something isn't working label Jan 29, 2022

tmszi mentioned this pull request Jan 29, 2022

mkhtml: Fix print warning/fatal message during compilation #2139

Merged

wenzeslaus added this to the 8.2.0 milestone Jan 29, 2022

wenzeslaus reviewed Jan 29, 2022

View reviewed changes

tmszi force-pushed the generate-local-pgms-json-file-with-last-commit branch from a768d3c to 29a7e7c Compare January 30, 2022 06:14

tmszi mentioned this pull request Feb 11, 2022

Problem installing addons #2186

Closed

tmszi added 6 commits February 13, 2022 13:52

GitHub action push to main branch

1137a32

Revert "GitHub action push to main branch"

f62abb0

This reverts commit 29a7e7c.

Revert "utils/mkhtml.py: generate local JSON file with PGMS and their…

d60c66e

… last commit" This reverts commit a3f56c0.

Make JSON file only instead of source code patch

28047cf

tmszi force-pushed the generate-local-pgms-json-file-with-last-commit branch from 718c945 to fc46089 Compare February 13, 2022 13:07

ninsbl mentioned this pull request Feb 14, 2022

scripts/g.extension: make GitHub REST API server rate limit error message more verbose #2193

Closed

ninsbl reviewed Feb 16, 2022

View reviewed changes

.github/workflows/gen_pgms_last_commit_json_file.yml Outdated Show resolved Hide resolved

ninsbl approved these changes Feb 16, 2022

View reviewed changes

wenzeslaus requested changes Feb 17, 2022

View reviewed changes

wenzeslaus requested changes Feb 19, 2022

View reviewed changes

tmszi added 2 commits February 19, 2022 10:33

Incorporate suggestions

75e35bb

utils/mkhtml.py: fix flake8 error unused imported module

98483b6

tmszi mentioned this pull request Feb 19, 2022

CI: Automatically create new GitHub release draft #2224

Merged

tmszi added 4 commits February 19, 2022 13:06

utils/generate_last_commit_file.py: improve catch *.html file

0ffd81d

Replace subprocess.check() func stdout and stderr param with capture_…

3f61f88

…output param

utils/generate_last_commit_file.py: replace unused dirs var with _

9ff2a65

Revert "Replace subprocess.check() func stdout and stderr param with …

82e7b3c

…capture_output param" This reverts commit 3f61f88. capture_output parmeter is supported by Python version >= 3.8

wenzeslaus requested changes Feb 20, 2022

View reviewed changes

utils/test_generate_last_commit_file.py Outdated Show resolved Hide resolved

utils/test_generate_last_commit_file.py Outdated Show resolved Hide resolved

utils/mkhtml.py Show resolved Hide resolved

utils/mkhtml.py Outdated Show resolved Hide resolved

tmszi added 2 commits February 20, 2022 08:19

Incorporate suggestions

de8157e

utils/test_generate_last_commit_file.py: fix flake8 error unused impo…

53d6276

…rted module

wenzeslaus approved these changes Mar 16, 2022

View reviewed changes

Use strict ISO 8601 date time format for git commit author date time

d2e88c7

tmszi requested a review from wenzeslaus April 13, 2022 19:43

wenzeslaus approved these changes Apr 14, 2022

View reviewed changes

tmszi merged commit 53ce4fd into OSGeo:main Apr 14, 2022

tmszi added the backport_needed label Apr 14, 2022

tmszi deleted the generate-local-pgms-json-file-with-last-commit branch April 14, 2022 03:50

wenzeslaus changed the title ~~utils/mkhtml.py: generate local JSON file with core modules and their last commit~~ mkhtml: Generate local JSON file with core modules and their last commit Apr 27, 2022

wenzeslaus mentioned this pull request Apr 29, 2022

CI: Add missing step id to release draft, pre-release, artifacts #2347

Merged

wenzeslaus removed the backport_needed label May 16, 2022

tmszi mentioned this pull request Dec 16, 2022

[Bug] FileNotFoundError for addons_paths.json #2706

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

mkhtml: Generate local JSON file with core modules and their last commit #2140

mkhtml: Generate local JSON file with core modules and their last commit #2140

tmszi commented Jan 29, 2022 •

edited

wenzeslaus left a comment

tmszi commented Jan 30, 2022 •

edited

lucadelu commented Feb 11, 2022

tmszi commented Feb 12, 2022

tmszi commented Feb 13, 2022

neteler commented Feb 13, 2022

lucadelu commented Feb 13, 2022

wenzeslaus commented Feb 14, 2022

tmszi commented Feb 14, 2022

ninsbl commented Feb 14, 2022

ninsbl left a comment

wenzeslaus left a comment

wenzeslaus Feb 16, 2022

wenzeslaus commented Feb 17, 2022

tmszi commented Feb 17, 2022 •

edited

wenzeslaus commented Feb 17, 2022

wenzeslaus left a comment

tmszi commented Feb 19, 2022

wenzeslaus left a comment

tmszi commented Feb 20, 2022

wenzeslaus left a comment

tmszi commented Apr 13, 2022

wenzeslaus commented May 16, 2022

mkhtml: Generate local JSON file with core modules and their last commit #2140

mkhtml: Generate local JSON file with core modules and their last commit #2140

Conversation

tmszi commented Jan 29, 2022 • edited

wenzeslaus left a comment

Choose a reason for hiding this comment

tmszi commented Jan 30, 2022 • edited

lucadelu commented Feb 11, 2022

tmszi commented Feb 12, 2022

tmszi commented Feb 13, 2022

neteler commented Feb 13, 2022

lucadelu commented Feb 13, 2022

wenzeslaus commented Feb 14, 2022

tmszi commented Feb 14, 2022

ninsbl commented Feb 14, 2022

ninsbl left a comment

Choose a reason for hiding this comment

wenzeslaus left a comment

Choose a reason for hiding this comment

wenzeslaus Feb 16, 2022

Choose a reason for hiding this comment

wenzeslaus commented Feb 17, 2022

tmszi commented Feb 17, 2022 • edited

wenzeslaus commented Feb 17, 2022

wenzeslaus left a comment

Choose a reason for hiding this comment

tmszi commented Feb 19, 2022

wenzeslaus left a comment

Choose a reason for hiding this comment

tmszi commented Feb 20, 2022

wenzeslaus left a comment

Choose a reason for hiding this comment

tmszi commented Apr 13, 2022

wenzeslaus commented May 16, 2022

tmszi commented Jan 29, 2022 •

edited

tmszi commented Jan 30, 2022 •

edited

tmszi commented Feb 17, 2022 •

edited