spec: https://download.copr.fedorainfracloud.org/results/fed500/include-what-you-use/fedora-rawhide-i386/05587380-iwyu/iwyu.spec srpm: https://download.copr.fedorainfracloud.org/results/fed500/include-what-you-use/fedora-rawhide-i386/05587380-iwyu/iwyu-0.19-1.fc39.src.rpm Description: Include what you use" means this: for every symbol (type, function, variable, or macro) that you use in foo.cc (or foo.cpp), either foo.cc or foo.h should include a .h file that exports the declaration of that symbol. (Similarly, for foo_test.cc, either foo_test.cc or foo.h should do the including.) Obviously symbols defined in foo.cc itself are excluded from this requirement. This puts us in a state where every file includes the headers it needs to declare the symbols that it uses. When every file includes what it uses, then it is possible to edit any file and remove unused headers, without fear of accidentally breaking the upwards dependencies of that file. It also becomes easy to automatically track and update dependencies in the source code. Fedora Account System Username: fed500 Would like to unretire this package.
Some initial comments reading through the spec file (not the formal review yet, I'll do that after these are addressed). 1) Why do you need to specify the LDFLAGS and the prefix path? Both those should be specified by the Fedora build flags properly. 2) I'd suggest modifying the way you exclude the assembly test to be by deleting the file in the prep section instead of modifying the configured CMake files. Additionally, it is more readable if you use a %ifnarch and just have one branch for the conditional. 3) `BuildRequires: python-devel` should be versioned, so it should be `BuildRequires: python3-devel` (according to the guidelines https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_distro_wide_guidelines) 4) For F36 and newer, I don't think you need to specify the compilers to CMake directly, it should find clang using the CC and CXX environment variables set by RPM.
> clang_major_ver=$(clang --version | grep version | cut -f3 -d" " | cut -f1 -d".") Use the %clang_major_version macro instead. This is defined in macros.clang which is part of clang-devel. > for test in ${failed_tests_add_include_argument[@]};do > sed -i "/===\/\//a\/\/ IWYU_ARGS: -I %{_libdir}/clang/${clang_major_ver}/include/" tests/cxx/$test > done > for test in ${failed_tests_additional_include_argument[@]};do > sed -i "s|-I .|-I . -I %{_libdir}/clang/${clang_major_ver}/include|g" tests/cxx/$test > done There is also a %clang_resource_dir amcro you can use here instead.
Maybe using %autorelease and %autochangelog macro for packaging has some benefits.
Thanks for the feedback. > 1) Why do you need to specify the LDFLAGS and the prefix path? Both those should be specified > by the Fedora build flags properly. Without LDFLAG do not get a position independent executable. Prefix path is suggested in install docs. > 2) I'd suggest modifying the way you exclude the assembly test to be by deleting the file in the > prep section instead of modifying the configured CMake files. Additionally, it is more readable if > you use a %ifnarch and just have one branch for the conditional. Done > 3) `BuildRequires: python-devel` should be versioned, so it should be > `BuildRequires: python3-devel` (according to the guidelines > https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_distro_wide_guidelines) Done > 4) For F36 and newer, I don't think you need to specify the compilers to CMake directly, > it should find clang using the CC and CXX environment variables set by RPM. Done > There is also a %clang_resource_dir macro you can use here instead. Done > Maybe using %autorelease and %autochangelog macro for packaging has some benefits. Prefer to manage manually for the time being, commits do not all correspond to changelog entries. spec: https://download.copr.fedorainfracloud.org/results/fed500/include-what-you-use/fedora-rawhide-x86_64/05588697-iwyu/iwyu.spec srpm: https://download.copr.fedorainfracloud.org/results/fed500/include-what-you-use/fedora-rawhide-x86_64/05588697-iwyu/iwyu-0.19-2.fc39.src.rpm
https://bugzilla.redhat.com/show_bug.cgi?id=2179648 *** This bug has been marked as a duplicate of bug 2179648 ***