-
Notifications
You must be signed in to change notification settings - Fork 36
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
Git checker doesn't get timestamps, resulting in unwanted PRs #154
Comments
Sure it would be a nice feature to have. |
The name of the branches that f-e-d-c pushes are based on the tree hash, which will change whenever the date changes, so if a PR is not merged within the same UTC day as being opened, it'd open another one because the hash would change. Maybe theoretical? Maybe acceptable? |
Didn't think about it. Yeah, that would be not great. |
Maybe we should add GitHub and GitLab checkers instead, that would use API? |
That could work. The GitLab API for fetching a tag's metadata is unauthenticated if the repo is public:
GitHub's appears to need authentication but happily the script already knows how to do that. (Was there a reason for shelling out to the |
Dunno. |
An alternate approach to prevent this would be to invent an algorithm to uniquely identify the update we made without relying on git commit hash to match. |
And yet another idea for workaround - maybe add |
This will automatically open PRs when new tags appear in the upstream repository. This uses the JSON checker rather than the Git checker because the Git checker cannot [currently][0] extract the timestamp from the Git tag, and would instead use today's date. fedc uses the Git tree hash to avoid duplicate PRs, so this would mean a new PR would be opened every day until one of them were merged. Unfortunately the JSON provided by GitLab does not include the date from the tag, only the date from the commit that the tag points to. As we will see in the subsequent commit, these are different, and it looks a bit weird that the release date is before the release name (itself a date). But this is relatively harmless, and importantly it is deterministic over time. [0]: flathub-infra/flatpak-external-data-checker#154
We have an in-memory representation of the manifest file(s) so I guess we could just hash those.
Thanks for adding this in #155. It works nicely. I will leave this ticket open as a wishlist item – it'd be less obscure to have the Git checker do the right thing by itself. Maybe I'll have a crack when I get time. |
This will automatically open PRs when new tags appear in the upstream repository. This uses the JSON checker rather than the Git checker because the Git checker cannot [currently][0] extract the timestamp from the Git tag, and would instead use today's date. fedc uses the Git tree hash to avoid duplicate PRs, so this would mean a new PR would be opened every day until one of them were merged. Unfortunately the JSON provided by GitLab does not include the date from the tag, only the date from the commit that the tag points to. As we will see in the subsequent commit, these are different, and it looks a bit weird that the release date is before the release name (itself a date). But this is relatively harmless, and importantly it is deterministic over time. [0]: flathub-infra/flatpak-external-data-checker#154
This will automatically open PRs when new tags appear in the upstream repository. This uses the JSON checker rather than the Git checker because the Git checker cannot [currently][0] extract the timestamp from the Git tag, and would instead use today's date. fedc uses the Git tree hash to avoid duplicate PRs, so this would mean a new PR would be opened every day until one of them were merged. Unfortunately the JSON provided by GitLab does not include the date from the tag, only the date from the commit that the tag points to. As we will see in the subsequent commit, these are different, and it looks a bit weird that the release date is before the release name (itself a date). But this is relatively harmless, and importantly it is deterministic over time. [0]: flathub-infra/flatpak-external-data-checker#154
The Git checker does not extract timestamps from tags so flathubbot will create a new pr on every daily run if the previous pr wasn't merged. See more details in f-e-d-c's bug report flathub-infra/flatpak-external-data-checker#154 To avoid this, switch to the KDE qtwebengine Gitlab mirror that can be used with the JSON checker. Also switch to pcituils tarball releases and add a check for python2.
The Git checker does not extract timestamps from tags so flathubbot will create a new pr on every daily run if the previous pr wasn't merged. See more details in f-e-d-c's bug report flathub-infra/flatpak-external-data-checker#154 To avoid this, switch to the KDE qtwebengine Gitlab mirror that can be used with the JSON checker. Also switch to pcituils tarball releases and add a check for python2.
This now also affects tarballs from GitHub, which now don't have persistent timestamps in HTTP |
Change x-data-checker to check the timestamp of updates, to avoid tons of duplicate pull requests if we don't merge changes straight away. See: flathub-infra/flatpak-external-data-checker#154
On a whim I wrote flathub/org.gnome.Boxes.Extension.OsinfoDb#2 to automatically update the database used by GNOME Boxes when it is tagged upstream. When reviewing my own PR I noticed that the date added in the appdata is today's date, rather than the date attached to the tag.
It doesn't seem to be possible to ask
git ls-remote
to include more commit metadata. I guess the "easiest" thing to do would be to make a shallow clone of the repository at exactly the tag in question, then read the timestamp out of the tag; but it would be nice to avoid having to fetch the commit contents. I don't know Git well enough to know if there is an operation that can do this. (The GitHub API has a way! But that's not useful in cases, like this one, which aren't on GitHub.)The text was updated successfully, but these errors were encountered: