Add release versions appstream files to increase_repos_version.sh
AbandonedPublic

Authored by jriddell on Jul 16 2019, 12:49 PM.

Details

Summary

Implements https://phabricator.kde.org/T10971

This adds the KDE Applications version to all projects. That makes sense since it's the version number used in the tars and so the version number user in the packaging.

But if it's not wanted on apps which prefer to use their own internal version number I'm not sure how to distinguish them. Check for cmake having a line like this?

project(bomber VERSION ${KDE_APPLICATIONS_VERSION})

Closes T10971

Diff Detail

Repository
R497 Release tools
Branch
add-appstream-version
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 16148
Build 16166: arc lint + arc unit
jriddell requested review of this revision.Jul 16 2019, 12:49 PM
jriddell created this revision.
aacid added a comment.Jul 16 2019, 1:25 PM

Each time you have proposed this i have pointed a script we have that gets the proper version from the cmake file.

Please go and check my previous comments.

If you can't find it i will point to it one more time.

jriddell updated this revision to Diff 62060.Jul 19 2019, 2:14 PM
jriddell edited the summary of this revision. (Show Details)

Add script to update appstream versions

Given this is going to the release-tools repo and the other code that also does this cmake trace thing to get the version is also in the release-tools repo do you think you could refactor it a bit to they share code?

If python is not your thing maybe @adrianchavesfernandez that did the "original" code can help you there?

jriddell updated this revision to Diff 62855.Jul 31 2019, 2:09 PM
  • move common functions to a common file
adrianchavesfernandez requested changes to this revision.Aug 5 2019, 11:04 AM
adrianchavesfernandez added inline comments.
add-appstream-versions/add-versions.py
34

Why are these values hardcoded here?

This revision now requires changes to proceed.Aug 5 2019, 11:04 AM
jriddell updated this revision to Diff 63193.Aug 6 2019, 2:07 PM
  • use the config file for config settings
add-appstream-versions/README.rst
6

Maybe we should modify both README files to simply point to

:code:`../modules.git`

While at it, this syntax is probably better:

``../modules.git``
15

I am guessing this is not what this script does.

add_versions_lib.py
33

You never imported Path in this file. Please, make sure your changes do work, there are no test and I’m likely to miss critical errors.

config
14

I’m not familar with the whole codebase, but it seems like we are simply moving the part where this data is hardcoded from a source file to a config file.

I think the simplest approach which does not imply hardcoding would be to accept these values as command-line parameters in the new script.

To make it better, we could:

  • Allow to alternatively specify APPSTREAM_UPDATER in a user-specific configuration file (one that is not Git-tracked).
  • Allow to pass the date in ISO format, and make the conversion to the desired format in the script itself.

Whatever the approach, the README should be updated accordingly.

This revision was not accepted when it landed; it landed in state Needs Review.Sep 5 2019, 7:04 PM
This revision was automatically updated to reflect the committed changes.
jriddell marked 3 inline comments as done.
jriddell reopened this revision.Sep 5 2019, 7:17 PM
jriddell marked an inline comment as done.
jriddell updated this revision to Diff 65477.Sep 5 2019, 7:17 PM
  • add appstream version script
  • move common functions to a common file
  • use the config file for config settings
  • update README, import path
  • fix imports
  • fix release date, update README

I'm not sure of the purpose in adding the option to use command line switches, the scripts already have that config file so seems easiest to reuse it.

There's no document with a good summary of how to do a release and no documentation of add-bugzilla-versions anywhere so I added them into README but it does feel like it could do with more description

aacid added a comment.Sep 5 2019, 7:27 PM

There's no document with a good summary of how to do a release

PACKAGING_HOWTO

can this be merged in?

Not as is, as it breaks add-bugzilla-versions/add-versions.py.

I proposed a patch against your branch which fixes it that @aacid already approved. Once I merge that, I’ll propose a similar patch to make it possible to use your new script as a simple Bash script. Then we can merge your branch into master.

How can progress be made on this? can you merge in the change to not break add-bugzilla-versions/add-versions.py ? Can you make the patch to make it possible to use your new script as a simple Bash script

I’ve just updated the add-appstream-version branch with the required changes.

I’ve tested --dry runs. @jriddell please test add_appstream_versions.sh in non-dry mode. @aacid, given the amount of changes it would not hurt to test add_bugzilla_versions.sh in non-dry mode as well.

If you are both OK with the state of both scripts in the add-appstream-version branch, add-appstream-version can be merged into master.

looks good to me, but I can't test it in non-dryrun mode since there's no release to be made, let's merge it in for the release

aacid added a comment.Oct 30 2019, 7:34 PM

Something is broken in add_bugzilla_versions.sh

(kdesrc) tsdgeos@xps:~/devel/kde/release-tools:add-appstream-version$ bash add_bugzilla_versions.sh 
Source folder: /home/tsdgeos/devel/kde
Email: aacid@kde.org
Password: 
kdialog
        (would have added 19.11.70)
                (error: RuntimeError("The version '19.11.70' already exists for product 'kdialog'."))
keditbookmarks
        (would have added 19.11.70)
                (error: RuntimeError("The version '19.11.70' already exists for product 'keditbookmarks'."))
kfind
        (would have added 19.11.70)
                (error: RuntimeError("The version '19.11.70' already exists for product 'kfind'."))
konqueror
        (no project version found)              Source folder: /home/tsdgeos/devel/kde/konqueror                /home/tsdgeos/devel/kde/konqueror/CMakeLists.txt does not define a project version              See https://community.kde.org/Guidelines_and_HOWTOs/Application_Versioning#Bugzilla_versions
Error: ProductRuntimeError('kate (error: cannot pull branch)\n\tSource folder: /home/tsdgeos/devel/kde/kate\n\tBranch: master')
aacid added a comment.Oct 30 2019, 7:36 PM

Something is broken in add_bugzilla_versions.sh

Ah, one problem was that i had local stuff in kate, but the stuff for konqueror still looks wrong, this used to be in various lines as far as i remember and not in a single one

the stuff for konqueror still looks wrong, this used to be in various lines as far as i remember and not in a single one

Solved.

aacid added a comment.Nov 1 2019, 10:54 AM

I guess you can commit but note i'm still unhappy about:

  • Depending on things that live on invent.kde.org:jriddell
  • Not being able to do a run that doesn't pushes things to the repo so we can just see the diff and see if it's breaking stuff or not

Merged!
The repo was moved, I've updated the comment for that now
There is a --dry operation for add_appstream_versions.sh

jriddell abandoned this revision.Nov 7 2019, 4:41 PM

merged not abandoned