search

sailfishos-chum / main

Documentation and issue tracker for the SailfishOS:Chum community repository
https://build.merproject.org/project/show/sailfishos:chum
MIT License
26 stars 4 forks source link

Submitting Storeman to the SailfishOS:Chum testing repository #77

Closed Olf0 closed 2 years ago

Olf0 commented 2 years ago

@rinigus, @poetaster contacted you with the wish to submit Storeman to the SailfishOS:Chum testing repository.

To prepare for this he replicated the setup of @mentaljam's Sailfish-OBS repo for Storeman in a sub-repo of @poetasters's Sailfish-OBS repo and I performed some minor adaptions. But I was strictly sticking to @mentaljam's OBS-repository structure, because this reflects the structure of Storeman's GitHub repository and the established workflow. Changing how Storeman's git- and OBS-repos are structured would require a fundamental change of the code organisation and the commit workflows, plus rewriting crucial parts of the spec file: This would be a tedious endeavour with the risk of breaking a well working, established processes, which were easy for me to comprehend in the few days since I started maintaining Storeman. TL;DR: I do think that the Storeman's git- and OBS-repo organisation and workflows @mentaljam established make a lot of sense.

@poetaster contacted you (via Telegram, I believe) and transported this feedback of yours:

\<rinigus> poetaster: can't you somehow organize it in a way where single master is pulled and options enabled/disabled depending on SFOS version?

Not easily. This would require a major reorganisation of code, git- and OBS-repo structure and workflows. It also would make the structure and processes much harder to understand.

\<rinigus> right now it is a pain to accept and start enabling/disabling all the sfos/arch combinations

When submitting a package, OBS does not include the the built target configuration, unfortunately.

But this can be easily done by Copy & Paste of the <build> section of the OBS meta-data / -tab from the package origin at OBS to its submitted location. The meta-data for the four packages harbour-storeman-installer, harbour-storeman-sfos3.2, harbour-storeman-sfos3.3, harbour-storeman-sfos4.2 and harbour-storeman-testing is at:

Alternatively I offer to create or configure the repository configuration for these packages, when you provide an initial set-up of these four packages and configure me as a maintainer.

On first sight it appears to be reasonable to put these four packages into a harbour-storeman sub-project (not just storeman, as above) of the SailfishOS:Chum testing repo (and later the regular SailfishOS:Chum repo), so they are kept together. But I am not very experienced with OBS, so there may be reasons why this is deemed a "bad idea™".

The maintainer's OBS-account names are poetaster, mentaljam and olf.

Thank you for your support!

rinigus commented 2 years ago

I managed to reject those submissions already as warned in the morning. So, if you will go for it, then you will have to resubmit it again. What I fail to understand is why not to use few #ifdef in the code and just provide correspondent macro during configuration stage of a package. Although, not sure it is my place to ask it.

rinigus commented 2 years ago

PS: Rejection was based on the perception that the work which was not done in the code packaging is pushed on Chum maintainers. So, instead of having single source code that is adjusted according to SFOS version, you prefer OBS maintainers to handle build config. It is good that you have provided build configs, but it is still copy&paste by us. This load can be shared in :testing, but not in :chum proper.

In addition, note that this will be requiring constant updates for those sfosX.Y versions with the addition of new SFOS versions at OBS (4.4.x.y coming up). So, we will have to maintain it with the each update of SFOS as well.

From such maintenance POV, I would like to ask you to reconsider and make the source workable for all SFOS versions using classical #ifdef and similar techniques.

poetaster commented 2 years ago

Frankly, I agree with rinigus. I think the workflow should be changed. The current diffs between SFOS versions are so trivial as to be managed with unified diffs. AND there are other considerations, ie. the application of version spanning changes that beg for a revision of the branch management.

I think it's 'ok', just, for the interim.

Olf0 commented 2 years ago

What I fail to understand is why not to use few #ifdef in the code and just provide correspondent macro during configuration stage of a package.

As stated,

This would require a major reorganisation of code, git- and OBS-repo structure and workflows. It also would make the structure and processes much harder to understand.

TL;DR: Because mentaljam did not organise code and workflows this way. Or read his own words.

Although, not sure it is my place to ask it.

You sure always have the freedom to ask.

PS: Rejection was based on the perception that the work which was not done in the code packaging is pushed on Chum maintainers. So, instead of having single source code that is adjusted according to SFOS version, you prefer OBS maintainers to handle build config.

Rinigus, I humbly but distinctively have to express at this point that this is too much of an accusation for me, in the light that I clearly stated:

Alternatively I offer to create or configure the repository configuration for these packages, when you provide an initial set-up of these four packages and configure me as a maintainer.

It is good that you have provided build configs, but it is still copy&paste by us. This load can be shared in :testing, but not in :chum proper.

This is just a matter of your trust in me (or not) to configure me as a package maintainer for these packages or sub-repository. Be it at SailfishOS:Chum testing and / or SailfishOS:Chum.

Also note that only three of these packages will be submitted from SailfishOS:Chum testing to SailfishOS:Chum proper: The three harbour-storeman-sfosX.Y release packages.

In addition, note that this will be requiring constant updates for those sfosX.Y versions with the addition of new SFOS versions at OBS (4.4.x.y coming up). So, we will have to maintain it with the each update of SFOS as well.

This is not true, but I am glad that you brought up this incorrect argument, which made me think. I should have inverted the <build> configuration for all packages except for harbour-storeman-sfos4.2: Defaults to off, and only on for the supported release targets.

So, if you will go for it, then you will have to resubmit it again.

So I will write most of the config anew and resubmit.

Olf0 commented 2 years ago

All right, next round!

The reworked meta-data (which has become much more compact) for the four packages is now at:

On first sight it appears to be reasonable to put these four packages into a harbour-storeman sub-project of the SailfishOS:Chum testing repo (and later the regular SailfishOS:Chum repo), so they are kept together. But I am not very experienced with OBS, so there may be reasons why this is deemed a "bad idea™".

Still I alternatively offer to create or configure the repository configuration for these packages, if you provide an initial, minimalistic set-up of these four packages and configure me as a maintainer; be it at SailfishOS:Chum testing and / or SailfishOS:Chum.

Note that only the three harbour-storeman-sfosX.Y release packages will be submitted from SailfishOS:Chum testing to SailfishOS:Chum proper.

The maintainer's OBS-account names are poetaster, mentaljam and olf.

piggz commented 2 years ago

I may be wrong, but https://github.com/storeman-developers/harbour-storeman/compare/sfos3.2...sfos3.3 shows basically no differences between 3.2 and 3.3 branch?

poetaster commented 2 years ago

I may be wrong, but storeman-developers/harbour-storeman@sfos3.2...sfos3.3 shows basically no differences between 3.2 and 3.3 branch?

There are a couple of small c++ changes which, think are of no consequence. That is 3.3 should work from < 4. But I've only read the changes and not tested. I don't have a device < 3.4 to test with.

rinigus commented 2 years ago

Re next round: I am not thrilled about merging this. It is fine if you prefer to keep that structure, for one reason or another. For Chum, I would expect one project to provide harbour-storeman and not the three or four different ones with the _meta doing its balancing. Let's see if @piggz would be fine merging it.

Re -testing: we don't have -testing in Chum - no point in submitting it. As SFOS version never resolves to -testing, but to the real SFOS version, there is no use in it either.

Re trust and maintainer: sure, you would be made a maintainer in :chum:testing as we do for all packages with the corresponding developers. So far, we have been able to manage :chum proper with @piggz as it is fortunately relatively low maintenance.

Olf0 commented 2 years ago

@piggz, I think I answered and resolved all criticism @rinigus has denoted above. If I missed something and left one of his statements unaddressed, please @piggz & @rinigus let me know.

As I am in the process of moving and consolidating infrastructure and workflows for Storeman, I considered having Storeman in SailfishOS:Chum to be elegant, because then not only the Storeman Installer can be used for installing Storeman, but also the SailfishOS:Chum GUI application. If you or @rinigus do not want Storeman in SailfishOS:Chum, this is a decision I will have to accept and then Storeman will continue to reside in a "private" OBS repo.

Because Storeman is building and working fine, the current goal is to move it to infrastructure which is not solely controlled by @mentaljam, plus consolidating (and defining formerly undefined) workflows for maintaining Storeman. Again, SailfishOS:Chum appeared to be an elegant solution to collaboratively build and publish Storeman.

But fundamentally altering Storeman's code organisation and workflows at GitHub is nothing I pursue, because I am convinced that @mentaljam's design is fine and working well, plus I do not have the time and motivation to implement, document and test any radical changes. If you want to pursue this, please pose a PR.

P.S.:

I may be wrong, […]

Besides some diverging code, the build targets (compiler version and libraries) do differ, hence they cannot be easily merged at GitHub. Actually, these differences are exactly the reasons why multiple release branches exist. Also see @mentaljam's statements (as the original author of Storeman and designer of its code organisation and workflows at GitHub) at [1], [2] and [3].

piggz commented 2 years ago

@Olf0 Ive no issue with Storeman being in SailfishOS:Chum, I merely questioned if you really needed that many packages, an instead could use tricks to have single version/codebase, which would have benefits going forward. As i said above, there are no real changes between branches 3.2 and 3.3, i know @poetaster said there was some code changes, but I dont see them :)

Ultimately, its your burden, so Im happy to accept what you submit.

I wonder if the -installer is needed, as that does a similar job to chum, deciding on the version to install?

And,as rinigus says, if you make all packges Provide harbour-storeman, if you fix up the builds in the future you will have a way to move forward.

Olf0 commented 2 years ago

@Olf0 Ive no issue with Storeman being in SailfishOS:Chum, […] Ultimately, its your burden, so Im happy to accept what you submit.

Thank you.

\<piggz> I wonder if the -installer is needed, as that does a similar job to chum, deciding on the version to install?

As I denoted earlier:

Also note that only three of these packages will be submitted from SailfishOS:Chum testing to SailfishOS:Chum proper: The three harbour-storeman-sfosX.Y release packages.

The Storeman Installer and Storeman-testing were submitted, because …

SailfishOS:Chum [testing] appear[s] to be an elegant solution to collaboratively build and publish Storeman.

I.e., in order not to use a "private" OBS repository (home:xyz) for this. The goal is to avoid exactly these hassles of moving build infrastructure in the future, if one of the Storeman maintainers ceases to participate. Hence SailfishOS:Chum testing looks as a suitable place to build all software the Storeman initiative releases.

\<rinigus> Re -testing: we don't have -testing in Chum - no point in submitting it. As SFOS version never resolves to -testing, but to the real SFOS version, there is no use in it either.

See the prior paragraph for the motivation for submitting it.

If you mean to address the sailfish_testing_<arch> targets used for Storeman-testing: That was in conjunction with point 2 of #76 and is the configuration I use in my private repo, because @mentaljam uses it in his and it makes sense to me. sailfish_testing_<arch> resolves to SailfishOS:latest, which technically more or less addresses all SFOS releases, in the sense that the built RPMs appear in the download directories for all SFOS releases, but are only build once against the most recent SailfishOS:a.b.c.d build target.

You are correct that the sailfish_testing_<arch> targets resolve to nothing, as long they are not defined in the global configuration of SailfishOS:Chum testing. As it is my interest, that Storeman-testing indeed becomes built, I will sure reconfigure this to build targets which actually work, until point 2 of #76 is implemented. Actually, this is exactly how it happened for the Storeman Installer yesterday.

Olf0 commented 2 years ago

Thanks to everybody who supported this task. From my perspective this was an important part of renewing the build and distribution infrastructure for Storeman.

Contrary to @piggz' suggestion above to leave the Storeman Installer out (which does make sense under technical aspects), I decided to leave the Storeman Installer in SailfishOS:Chum, when asked by @rinigus to either have both in SailfishOS:Chum proper and testing, or not at all. Ultimately I am quite happy with this decision, because this allows for easy testing of both, the Storeman Installer and Storeman proper on new SailfishOS releases via the SailfishOS:Chum GUI application, before the "download on demand (DoD)" md-rpm repositories for a new SFOS release has been published by Jolla at the SFOS-OBS (even during cBeta and EA periods).

For the aspects of reorganising Storeman's code in order to eliminate the release branches, I refrained to discuss this topic in depth, because:

The next endeavour I am planning is to improve Storeman Installer, from which the SailfishOS:Chum GUI application might also benefit by eliminating the tedious procedure to install it at the GUI. This may take a while (as time and other tasks permit, plus "outside season" has started), but seems to be well within my capabilities.

I would be happy if anyone reorganises Storeman's source code into a single branch, while keeping all its target releases and current functionality intact (i.e., simplistic approaches like "ditch all divergent code and let the SFOS 3.1/3.2 target die" are still not welcome) plus obtaining a comprehensible and well maintainable result.

Cheers!

Olf0 commented 2 years ago

A last question in the context of "submitting Storeman to the SailfishOS:Chum testing repository", but addressing OBS behaviour.

Observations

Questions

P.S.: The issue report closest to this phenomenon I was able to find is OBS issue 5192, but a couple of details are not matching, while the basics look similar.

rinigus commented 2 years ago

If I remember correctly, tar_git calculates release as a part of the service. I guess you should be able to get shorter version if you tag the commit that you build.

There is a way to tell OBS "you must not alter version numbers", by enforcing release in project config. This is not done by default as it is useful to have increments of release when the build is updated due to some of the dependency rebuild.

So, in short, try to tag the commits and trigger new builds. That should help you out.

Olf0 commented 2 years ago

If I remember correctly, tar_git calculates release as a part of the service.

Unfortunately not only that, but also the version, as described above.

I guess you should be able to get shorter version if you tag the commit that you build.

"Shorter" is not the point, "correct" is.

So, in short, try to tag the commits and trigger new builds. That should help you out.

I already do, as described above!

I once tried a commit short-hash, which yielded exactly the same result. So tagging makes no difference at all (with OBS' default settings).

There is a way to tell OBS "you must not alter version numbers", by enforcing release in project config. This is not done by default as it is useful to have increments of release when the build is updated due to some of the dependency rebuild.

I finally found something. But it is not really a documentation: https://github.com/openSUSE/obs-service-tar_scm/blob/master/tar_scm.service.in

Does anybody know a better explanation how this works in practice (which would spare me a lot of trial and error)?

rinigus commented 2 years ago

Unfortunately not only that, but also the version, as described above.

Yes, sorry. Indeed, full version string is set based on tag and your commit you build against. Release is set internally to ensure updates.

So tagging makes no difference at all (with OBS' default settings).

I don't think it is that, but maybe could be tried. Tags are expected to have a form without dashes as dash is something that separates version and release. So, for example v0.3.0.2.sfos4.2. Try to add a tag in such format and see if it helps.

It is possible that tar_git does not expect tags in branches and calculates its release based on the last tag in the main branch and then adds commit info.

Note that tar_scm is something different. tar_git. It is from https://github.com/MeeGoIntegration/obs-service-tar-git

Olf0 commented 2 years ago

So tagging makes no difference at all (with OBS' default settings).

I don't think it is that, but maybe could be tried. Tags are expected to have a form without dashes as dash is something that separates version and release.

Yes, no dashes allowed, see https://github.com/MeeGoIntegration/obs-service-tar-git/blob/master/tar_git#L406

So, for example v0.3.0.2.sfos4.2. Try to add a tag in such format and see if it helps.

It does "help", but that crazy version string ends up in the names of the built RPMs. But as "dumb mode" is really dumb, this may be the best result which is achievable.

It is possible that tar_git does not expect tags in branches and calculates its release based on the last tag in the main branch and then adds commit info.

Which is wrong (i.e., a bug), when one explicitly specifies a branch.

Note that tar_scm is something different. tar_git. It is from https://github.com/MeeGoIntegration/obs-service-tar-git

Thank you! That was very helpful. First I was happy that it is a shell script, but it is so convoluted and without any comments, that it hard to fully understand it.

But it is OBS, I should be able to use tar_scm instead, right? No: Files could not be expanded: service error: 400 remote error: /usr/lib/obs/service//tar_scm.service No such file or directory.

Olf0 commented 2 years ago

Note that tar_scm is something different. tar_git. It is from https://github.com/MeeGoIntegration/obs-service-tar-git

Thank you! That was very helpful. First I was happy that it is a shell script, but it is so convoluted and without any comments, that it hard to fully understand it.

I went through it until I fully understood the parsing of tags:

  1. A valid tag for tar_git may contain at most a single dash ("-"). The sub-string starting at the dash is discarded.
  2. A valid tag for tar_git may be prefixed by either a small "v" or a "vendor string" ending in a slash ("/"). The "v" or the "vendor string" including the slash are discarded.
  3. The resulting, stripped tag must conform to the extended regular expression (ERE) ^[[:digit:]]+\.[[:alnum:].~+]*$ (but is actually written much more complicated) and originate from the branch provided by the branch parameter, otherwise it is discarded.
  4. If there is no valid tag or if a commit ID (short-hash) was provided, tar_git looks for the "closest" tag which fulfils aforementioned rules (i.e., runs through steps 1 to 3 with it), but misses to filter for the branch provided by the branch parameter. It then appends a date string and a short-hash, which are a clear indication that this code path was used.

Because my release tags always contained two dashes, they were discarded and the alternate code path found v0.2.12-master as then newest "valid" tag (although referring to the master branch) and used it.

In the future I will use, e.g., sfos4.2/0.3.0-2 as a tag name, which results in, e.g., harbour-storeman-0.3.0-1.13.1.jolla.armv7hl.rpm.

P.S.: When the token parameter contains a non-empty string (not a regular expression), this is used as a sub-string a valid tag must match to, otherwise the tag is discarded. Hence I may use sfos as a token.

piggz commented 2 years ago

If it helps, I always use tags in the format x.y.x+somethingN, eg, 1.2.3+git1

Olf0 commented 2 years ago

If it helps, I always use tags in the format x.y.x+somethingN, eg, 1.2.3+git1

Yeah, once understood how it works, one can play with it: For 1.2.3+git1, 1.2.3~git1 and 1.2.3.git1 the whole string will become part of the RPM file-name, for 1.2.3-git1, git1/1.2.3 and foo/1.2.3-bar only the 1.2.3.

But I missed to state my requirements:

This is why I am actually quite happy with, e.g., sfos4.2/0.3.0-2 → harbour-storeman-0.3.0-1.13.1.jolla.armv7hl.rpm.

Olf0 commented 2 years ago

Thank you @rinigus for the hints and pointers WRT tar_git, which resulted in a much more sophisticated git-tag naming scheme for Storeman and ultimately in nice RPM file names for Storeman at the SailfishOS-OBS, including SailfishOS:Chum.

I also discovered a packaging bug I introduced, which affected older SFOS versions and prevented Storeman being offered for installation or upgrading for them.

All this is now resolved with Storeman 0.3.0-4.

Please do update the package descriptions, which have been reworked (I often need some time to settle things, but now they are settled), because OBS does not copy them as part of a package submission. This is most easily done by Copy&Paste of the <description> section from and to the metadata tab:

  1. https://build.sailfishos.org/package/meta/sailfishos:chum:testing/harbour-storeman-installer → https://build.sailfishos.org/package/meta/sailfishos:chum/harbour-storeman-installer
  2. https://build.sailfishos.org/package/meta/sailfishos:chum:testing/harbour-storeman-sfos3.2 → https://build.sailfishos.org/package/meta/sailfishos:chum/harbour-storeman-sfos3.2
  3. The description of harbour-storeman-sfos3.3 was not altered.
  4. https://build.sailfishos.org/package/meta/sailfishos:chum:testing/harbour-storeman-sfos4.2 → https://build.sailfishos.org/package/meta/sailfishos:chum/harbour-storeman-sfos4.2

Thanks again, with this last step I believe the task "Submitting Storeman to SailfishOS:Chum" will be completely done.

Olf0 commented 1 year ago

FYI: As Storeman Installer 2 works without user interaction ("unattended"), there is no longer a reason for the three Storeman variants to be individually distributed via SailfishOS:Chum in addition to the Storeman Installer.

Hence I suggest the deletion of the three Storeman variants at SailfishOS:Chum per requests 2558, 2559 and 2560 at the Sailfish-OBS. I already deleted them at SailfishOS:Chum:Testing.