-
Notifications
You must be signed in to change notification settings - Fork 17
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
Set package version without requiring to do manual steps before building #46
Comments
You should install the kernel modules before you build this package and don't reuse targets/snapshots for different device variants. |
Yes, the package is installed before the build. But, it shouldn't be needed for just parsing RPM SPEC using Notice the presence of Notice that during the build, version is determined correctly as a correct device kernel modules are pulled in. So, the issue comes when having xz2/xz2c/xz3 in the same shared RPMs folder, in the config as in OBS. As these devices require different kernels. I cannot have them installed at the same time if they have the same version due to the conflicts. |
@rinigus Try if processing the spec with
For me it worked with a regular spec file. |
@martyone , this doesn't choke anymore, but result is not super useful (and I keep this as an example of some weird rpm magic with
So, here we get unprocessed By making the proposed change, no such issues will occur. |
Yes this done because the version is tagged in the device repo, the proper fix would be to use the version from the git tag locally too in case it can't be determined earlier. |
I think it will be a case of over-engineering and could have more negative aspects without any positive. Iff we set the version by git tag in this case, we will pretend that this version carries some information. In this particular case, it is useless, as the repository version stays the same while kernels are updated. So, by adding a code determining the version from git would lead next developers to think that it is correct and could work instead. Which it does not. In case of OBS, I presume that it is using its own SPEC parser. My proposed patch makes SPEC parsed by regular As this left unanswered and to avoid confusion:
TBuilder is not force resetting target snapshots before building each package. It is a bit hidden in the code, but such reset occurs before the build for a package is started. After successful build, snapshot is reset with force option and the next package is built. |
Nope tar_git overrides the version so 0.1 gets replaced by the correct version that's why I said go with using git tag (see official devices). |
I must be blind. Yesterday I was able to reproduce the issue, now I am not. Nothing changed to my environment and the terminal's scrollback provides clear evidence it used to fail before. a.spec:
b.spec:
query it:
and also:
What am I doing wrong? :) |
@martyone: the section I have issue with is
that one requires
does it mean that you tag https://github.com/mer-hybris/droid-hal-img-boot-sony-seine for every kernel update? As that is a repo that we build over here. I am probably missing something with OBS in this case... |
As I forgot to reply to this: I am parsing with
to ensure that all included files are included properly. In this case, |
@rinigus yeah that made the difference in my case - I worked with the kernel-modules package before, then I switched to the kernel-headers in order to be able testing different aspects of the evaluation and forgot about. It's really a chicken-egg problem. Wouldn't this work? -%if 0%{?_obs_build_project:1}
-Version: 0.0.1
+# This needs droid-hal-%%{device}-kernel-modules but that is only pulled in as BuildRequires.
+%if 0%{!?localver:1}
+Version: 999.999.999.999
%else
Version: %{localver}
%endif |
we also have {localver} used for So, to handle it in a simple manner, I propose to check if localver evaluates into empty string and fill it with something in this case. As in submitted PR #47 |
Damn, misread Provides statement. Probably your approach will work as well, but 0%{!?localver:1} would have to be replaced with something. Let me check it to be sure, sorry for rushing with reply |
Found the reason for confusion: on Tama, I set So, for me, it is better to define localver to something meaningful. For upstream, I guess your approach would work after fixing %if statement When using
it fails for me (error: line 66: Empty tag: Version:). With
it works. To reduce the diff between Tama and upstream, would be nice to have |
Given that there is "Version: %{localver}", wouldn't this work equally?
(Assumed you do not need to do |
It would for non-OBS builds. With OBS, you end up with the versions like And provides are determined from RPM binaries, as we have some that are generated ( |
The version is set but rebuild with a new pkgrel every time a dependency updates. |
@Thaodan - I guess the new title is more of a side-effect of the requested change :) . |
The requested change did not describe the actual issue. Just adding some
version there to let RPM "parse" the right fix is a hack around the issue.
Requiring the install of dependencies prior build manually is hack in
packaging terms even ignoring the target might being unclean or any other
applied hacks somewhere else.
We should find a fix for that just let mb2/tar_git set the version from git.
|
Note that In principle, I think RPM SPEC should be readable in a form that it passes rpmspec without dependencies and using only tools that are installed in SDK by default. Otherwise it is a bug that should be fixed on SPEC side. How else are we expected to get requirements for the build? |
In principle, I think RPM SPEC should be readable in a form that it passes
rpmspec without dependencies and using only tools that are installed in SDK
by default. Otherwise it is a bug that should be fixed on SPEC side. How
else are we expected to get requirements for the build?
The packaged is tied to the kernel if you go further than this this package
would be part of droid-hal-kernel but since that is not practically we need to
use workarounds.
|
When mb2 is used for building, the Version gets substituted by mb2 based on Git tag the same way as tar_git does (the '--fix-version' option is implied when sources are under Git control). So, provided that the tags match the tags under the kernel package, there is no need for working with git at .spec level and the following should just work: -%if 0%{?_obs_build_project:1}
-Version: 0.0.1
-%else
-Version: %{localver}
-%endif
+Version: 0 Leaving |
@martyone: git tag for that specific package is not usually the kernel version. So, use of localver is justified perfectly in this case. But we can replace 0.0.1 by 0 as an indication. Assuming nothing else breaks with it |
hm, I was told the tags do match :) Anyway it feels odd when sources are tagged with different version than the version used for binaries, doesn't? So if they really don't match (in some cases), maybe it's a good opportunity to align them. But take me with a grain of salt, I have no idea of the actual tagging practice here. All I am working with is the bit of information I got indirectly, so my suggestions may be completely wrong. |
So, let's check:
Problem is that we don't have to change that particular package (droid-hal-img-boot) with the change in kernel version. So, it is easy to get the situation where droid-hal-img-boot repo stays the same while kernel is updated. But, you would want it to indicate the kernel version as well. Similar state is with PA module packages, if I understand correctly. There, version is taken from PA itself while release indicates state of the module. |
Fixed with PR #50 |
In the condition where
droid-hal-%{device}-kernel-modules
is absent in the target,rpmspec
cannot parsedroid-hal-XXX-img-boot.spec
as includeddroid-hal-device-img-boot.inc
doesn't defineVersion
in these conditions.I suggest to ensure that
localver
macro has always some value defined. For example, after https://github.com/mer-hybris/hybris-initrd/blob/master/droid-hal-device-img-boot.inc#L54 add something likeNot an expert in SPEC, please feel free to suggest something better.
Context: processing SPECs in the repository covering multiple devices with the different kernels.
The text was updated successfully, but these errors were encountered: