Add clang compile support #317

Please review the following changes and merge at your earliest convenience:

Add check for installed compiler, remove BuildRequires
Add platform option for clang when the default symlink resolves to it
Add OpenMandriva specific checks for required files removed during
packaging
Comment missing docs, install, and headers during packaging process
Increment version number to latest stable release
Replace symlink for build-examples with the actual file, install dereferences
Add missing translation file for "tr"
Fix lib prefixes for dependencies needed in MDK/OMV for 64 bit

This spec builds a srpm and rpms successfully in OMV Cooker.

Please review the following changes and merge at your earliest convenience: - Add check for installed compiler, remove BuildRequires - Add platform option for `clang` when the default symlink resolves to it - Add OpenMandriva specific checks for required files removed during packaging - Comment missing docs, install, and headers during packaging process - Increment version number to latest stable release - Replace symlink for build-examples with the actual file, `install` dereferences - Add missing translation file for "tr" - Fix lib prefixes for dependencies needed in MDK/OMV for 64 bit This spec builds a `srpm` and `rpms` successfully in OMV Cooker.

Cleaned up and ready for review. Apologies for the FP spam.

Will leave this to @Francois to review, since he is the TDE's RPM guru 😄

Note when you create PR: please add TDE/core and TDE/owners as reviewers, this will include the whole TDE core team, rather than just Slavek and myself. Then if there is anything RPM specific, add Francois as well.

Will leave this to @Francois to review, since he is the TDE's RPM guru 😄 Note when you create PR: please add TDE/core and TDE/owners as reviewers, this will include the whole TDE core team, rather than just Slavek and myself. Then if there is anything RPM specific, add Francois as well.

Note when you create PR: please add TDE/core and TDE/owners as reviewers, this will include the whole TDE core team, rather than just Slavek and myself. Then if there is anything RPM specific, add Francois as well.

I attempted to fix this, but the three of you are still reviewers. I would expect an admin could probably do it.

> Note when you create PR: please add TDE/core and TDE/owners as reviewers, this will include the whole TDE core team, rather than just Slavek and myself. Then if there is anything RPM specific, add Francois as well. I attempted to fix this, but the three of you are still reviewers. I would expect an admin could probably do it.

No big deal anyway, more just a guideline for future. I fixed that anyway

No big deal anyway, more just a guideline for future. I fixed that anyway

Thank you.

> No big deal anyway, more just a guideline for future. I fixed that anyway Thank you.

I'd also like to add that some of the changes in this PR fix a failure to package due to missing files that were present in the previous version. This would mean that rpm packaging could be failing currently.

@Francois were you able to confirm a good build for other rpm systems?

Below are some comments to consider. At the same time, I noticed that François probably did not merge the last changes to master branch, but he only pushed it to r14.1.x. This makes sense to compare your modifications with a solution that is already used in r14.1.x branch and solve it in the same way.

Below are some comments to consider. At the same time, I noticed that François probably did not merge the last changes to master branch, but he only pushed it to r14.1.x. This makes sense to compare your modifications with a solution that is already used in r14.1.x branch and solve it in the same way.

I will attempt to rebase with r14.1.x.

> Below are some comments to consider. At the same time, I noticed that François probably did not merge the last changes to master branch, but he only pushed it to r14.1.x. This makes sense to compare your modifications with a solution that is already used in r14.1.x branch and solve it in the same way. I will attempt to rebase with r14.1.x.

Doing a pull of the r14.1.x branch seems to encounter some issues with the gentoo submodule. I manually set it to the same branch in the submodule and attempted to update it but no changes are made. I'm sure I am missing something with it as I never have good luck with submodules. Basically, the rebase always says there are changes in the gentoo folder under git status but adding and committing doesn't resolve it when I try to do git rebase --continue. I may just start over from a new branch based on r14.1.x instead of master. It would be much simpler. I feel like I would have typed the changes several times over trying to resolve the issue and typing this reply. :)

Doing a pull of the `r14.1.x` branch seems to encounter some issues with the `gentoo` submodule. I manually set it to the same branch in the submodule and attempted to update it but no changes are made. I'm sure I am missing something with it as I never have good luck with submodules. Basically, the rebase always says there are changes in the `gentoo` folder under `git status` but adding and committing doesn't resolve it when I try to do `git rebase --continue`. I may just start over from a new branch based on `r14.1.x` instead of `master`. It would be much simpler. I feel like I would have typed the changes several times over trying to resolve the issue and typing this reply. :)

Hello @zeroability , sorry the master branch was outdated, all previous commits I did were in r14.1.x branch, so I've retrieved them in master branch.

About the 'clang/gcc' switch:
I don't think that adding a fixed 'distribution -> compiler' association here, such as 'if openmandriva then clang, else gcc', is a good idea. Several distributions offer both clang and gcc, I think it should be a global choice for the person building the package, and that choice applies to all packages.
Since we used gcc by default, we can also consider that undefined %c_compiler macro means using gcc.
I suggest we keep usage of %c_compiler macro as you wrote, except for the %global definition. In my case, I would put this definition in the $HOME/.rpmmacros file.
Also, I prefer keeping an explicit 'BuildRequires' for gcc-c++ or clang, depending in the %c_compiler value.

Same for other rpmbuild macros such as __brp_remove_la_files : I put them in .rpmmacros, not in spec files.

About the 'BuildRequires' changes:
I've already updated them so that it works on all distributions, including openmandriva. So your changes should not be necessary.

About the 'build-examples.sh' symlink:
In my case, a build script copies all sources files in a temporary directory, resolving symlinks, before running actual rpmbuild command.
But when running rpmbuild directly, I think rpmbuild will embed the symlink instead of actual file in the build process, causing failure.
I'm note sure what is the best way to solve this. I'd prefer not duplicating the file.

I'm testing on my side, I will update the PR afterwards.

Hello @zeroability , sorry the master branch was outdated, all previous commits I did were in r14.1.x branch, so I've retrieved them in master branch. About the 'clang/gcc' switch: I don't think that adding a fixed 'distribution -> compiler' association here, such as 'if openmandriva then clang, else gcc', is a good idea. Several distributions offer both clang and gcc, I think it should be a global choice for the person building the package, and that choice applies to all packages. Since we used gcc by default, we can also consider that undefined %c_compiler macro means using gcc. I suggest we keep usage of %c_compiler macro as you wrote, except for the %global definition. In my case, I would put this definition in the $HOME/.rpmmacros file. Also, I prefer keeping an explicit 'BuildRequires' for gcc-c++ or clang, depending in the %c_compiler value. Same for other rpmbuild macros such as __brp_remove_la_files : I put them in .rpmmacros, not in spec files. About the 'BuildRequires' changes: I've already updated them so that it works on all distributions, including openmandriva. So your changes should not be necessary. About the 'build-examples.sh' symlink: In my case, a build script copies all sources files in a temporary directory, resolving symlinks, before running actual rpmbuild command. But when running rpmbuild directly, I think rpmbuild will embed the symlink instead of actual file in the build process, causing failure. I'm note sure what is the best way to solve this. I'd prefer not duplicating the file. I'm testing on my side, I will update the PR afterwards.

About the 'clang/gcc' switch:
I don't think that adding a fixed 'distribution -> compiler' association here, such as 'if openmandriva then clang, else gcc', is a good idea. Several distributions offer both clang and gcc, I think it should be a global choice for the person building the package, and that choice applies to all packages.
Since we used gcc by default, we can also consider that undefined %c_compiler macro means using gcc.
I suggest we keep usage of %c_compiler macro as you wrote, except for the %global definition. In my case, I would put this definition in the $HOME/.rpmmacros file.
Also, I prefer keeping an explicit 'BuildRequires' for gcc-c++ or clang, depending in the %c_compiler value.

Same for other rpmbuild macros such as __brp_remove_la_files : I put them in .rpmmacros, not in spec files.

This came from a discussion with @bero. Package building should imply that a default compiler would be used in the majority of circumstances. Setting a BuildRequires for it would be redundant. Doing conditional checks for what the default is and then assigning a BuildRequires for it would also make no sense in this context. This is really a temporary fix until the CMake project can be completed.

About the 'BuildRequires' changes:
I've already updated them so that it works on all distributions, including openmandriva. So your changes should not be necessary.

I appreciate that.

About the 'build-examples.sh' symlink:
In my case, a build script copies all sources files in a temporary directory, resolving symlinks, before running actual rpmbuild command.
But when running rpmbuild directly, I think rpmbuild will embed the symlink instead of actual file in the build process, causing failure.
I'm note sure what is the best way to solve this. I'd prefer not duplicating the file.

This may be specific to our build/package process, or the version of coreutils we are using. Something else @bero could probably speak to. I would agree with not using a symlink if there is no guarantee or checking that the file actually exists. It might make more sense that if the file is needed for more than one distribution that it be moved to a new directory in the tde-packaging directory so that it can have a real path across all package/build recipes instead of using a symlink. Something like tde-packaging/scripts/tqt3/build-examples.sh. Any other distribution independent scripts should also be moved to a more sensible place like this.

Thank you for helping out with this.

>About the 'clang/gcc' switch: I don't think that adding a fixed 'distribution -> compiler' association here, such as 'if openmandriva then clang, else gcc', is a good idea. Several distributions offer both clang and gcc, I think it should be a global choice for the person building the package, and that choice applies to all packages. Since we used gcc by default, we can also consider that undefined %c_compiler macro means using gcc. I suggest we keep usage of %c_compiler macro as you wrote, except for the %global definition. In my case, I would put this definition in the $HOME/.rpmmacros file. Also, I prefer keeping an explicit 'BuildRequires' for gcc-c++ or clang, depending in the %c_compiler value. >Same for other rpmbuild macros such as __brp_remove_la_files : I put them in .rpmmacros, not in spec files. This came from a discussion with @bero. Package building should imply that a default compiler would be used in the majority of circumstances. Setting a `BuildRequires` for it would be redundant. Doing conditional checks for what the default is and then assigning a `BuildRequires` for it would also make no sense in this context. This is really a temporary fix until the CMake project can be completed. >About the 'BuildRequires' changes: I've already updated them so that it works on all distributions, including openmandriva. So your changes should not be necessary. I appreciate that. >About the 'build-examples.sh' symlink: In my case, a build script copies all sources files in a temporary directory, resolving symlinks, before running actual rpmbuild command. But when running rpmbuild directly, I think rpmbuild will embed the symlink instead of actual file in the build process, causing failure. I'm note sure what is the best way to solve this. I'd prefer not duplicating the file. This may be specific to our build/package process, or the version of `coreutils` we are using. Something else @bero could probably speak to. I would agree with not using a symlink if there is no guarantee or checking that the file actually exists. It might make more sense that if the file is needed for more than one distribution that it be moved to a new directory in the `tde-packaging` directory so that it can have a real path across all package/build recipes instead of using a symlink. Something like `tde-packaging/scripts/tqt3/build-examples.sh`. Any other distribution independent scripts should also be moved to a more sensible place like this. Thank you for helping out with this.

I just wanted to check the status of this since there hasn't been any updates in a few months. If I was supposed to do something more with this, I apologize for misunderstanding. My new job keeps me very busy. Thank you.

Hello, sorry I've just slightly modified your PR, as stated in my previous comment.
I agree to merge as-is.

Hello, sorry I've just slightly modified your PR, as stated in my previous comment. I agree to merge as-is.

@Francois I will live this to you to merge once you and @zeroability are both happy with it.

 @ -51,7 +59,6 @@ Source1:	build-examples.sh
 Source2:	trinity-tqt3-rpmlintrc
 BuildRequires: glibc-devel
 BuildRequires: gcc-c++

 @ -65,3 +72,3 @@
 # JPEG support
 BuildRequires: libjpeg-devel
 BuildRequires: %{_lib}jpeg-devel

 @ -68,3 +75,3 @@
 # MNG support
 BuildRequires: libmng-devel
 BuildRequires: %{_lib}mng-devel

 @ -71,3 +78,3 @@
 # PNG support
 BuildRequires: libpng-devel
 BuildRequires: %{_lib}png-devel

 @ -737,1 +747,3 @@
 %{_includedir}/tqt3/qttableview.h
 # not found during packaging - OMV
 #%%{_includedir}/tqt3/qtmultilineedit.h
 #%%{_includedir}/tqt3/qttableview.h

Labels Milestones

Add clang compile support #317

Reviewers

				`@ -1 +0,0 @@`
				`../../../debian/_base/dependencies/tqt3/debian/maintain/build-examples.sh`

 @ -916,1 +927,3 @@
 %{_docdir}/tqt3-compat-headers/attic.tar.gz
 # not found during packaging, OMV
 #%%dir %%{_docdir}/tqt3-compat-headers
 #%%{_docdir}/tqt3-compat-headers/attic.tar.gz

 @ -1343,0 +1357,4 @@
 %if "%{?c_compiler}" == "clang"
     -platform linux-clang \
 %else
 %if "%{?c_compiler}" == "gcc"

 @ -1470,0 +1491,4 @@
 # not found in 14.1.1
 #pushd src
 #tar cvvfz "attic.tar.gz" attic/
 #install -D -m644 "attic.tar.gz" "%{?buildroot}%{_docdir}/tqt3-compat-headers/attic.tar.gz"