-
Notifications
You must be signed in to change notification settings - Fork 4
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
Submitting Storeman to the SailfishOS:Chum testing repository #77
Comments
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. |
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. |
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. |
As stated,
TL;DR: Because mentaljam did not organise code and workflows this way. Or read his own words.
You sure always have the freedom to ask.
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:
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
This is not true, but I am glad that you brought up this incorrect argument, which made me think. I should have inverted the
So I will write most of the config anew and resubmit. |
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 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 |
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. |
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 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. |
@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.:
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]. |
@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. |
Thank you.
As I denoted earlier:
The Storeman Installer and Storeman-testing were submitted, because …
I.e., in order not to use a "private" OBS repository (
See the prior paragraph for the motivation for submitting it. If you mean to address the You are correct that the |
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! |
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. |
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. |
Unfortunately not only that, but also the version, as described above.
"Shorter" is not the point, "correct" is.
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).
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)? |
Yes, sorry. Indeed, full version string is set based on tag and your commit you build against. Release is set internally to ensure updates.
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 |
Yes, no dashes allowed, see https://github.com/MeeGoIntegration/obs-service-tar-git/blob/master/tar_git#L406
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.
Which is wrong (i.e., a bug), when one explicitly specifies a branch.
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:
Because my release tags always contained two dashes, they were discarded and the alternate code path found In the future I will use, e.g., P.S.: When the |
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 But I missed to state my requirements:
This is why I am actually quite happy with, e.g., |
Thank you @rinigus for the hints and pointers WRT 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
Thanks again, with this last step I believe the task "Submitting Storeman to SailfishOS:Chum" will be completely done. |
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. |
@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:
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.
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 juststoreman
, 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
andolf
.Thank you for your support!
The text was updated successfully, but these errors were encountered: