Spec URL: https://sergiomb.fedorapeople.org/7zip/7zip.spec SRPM URL: https://sergiomb.fedorapeople.org/7zip/7zip-24.09-1.fc43.src.rpm Description: This package contains the 7z command line utility for archiving and extracting various formats. Fedora Account System Username: sergiomb This is a Rename request for the former package 'p7zip'
This package built on koji: https://koji.fedoraproject.org/koji/taskinfo?taskID=129324662
The ticket summary is not in the correct format. Expected: Review Request: <main package name here> - <short summary here> Found: Rename Request: 7zip - Command-line file archiver with high compression ratio As a consequence, the package name cannot be parsed and submitted to be automatically build. Please modify the ticket summary and trigger a build by typing [fedora-review-service-build]. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
[fedora-review-service-build]
*** Bug 2346040 has been marked as a duplicate of this bug. ***
Copr build: https://copr.fedorainfracloud.org/coprs/build/8662107 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2346041-7zip/fedora-rawhide-x86_64/08662107-7zip/fedora-review/review.txt Found issues: - Not a valid SPDX expression 'BSD-3-Clause AND LGPL-2.1-or-later AND SUSE-Public-Domain'. Read more: https://fedoraproject.org/wiki/Changes/SPDX_Licenses_Phase_1 Please know that there can be false-positives. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
Created attachment 2076805 [details] The .spec file difference from Copr build 8662107 to 8662184
Copr build: https://copr.fedorainfracloud.org/coprs/build/8662184 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2346041-7zip/fedora-rawhide-x86_64/08662184-7zip/fedora-review/review.txt Please take a look if any issues were found. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
Taking this.
per guideline https://docs.fedoraproject.org/en-US/fesco/Package_review_policy/#reviewer_not_responding , the bug was reset
Sorry what? You didn't even ping me or anything. It hasn't even been a week, much less a month.
"If there is no response within one week, the fedora‑review flag is set to the empty value." can you start the review ? or let someone else review it
can you please start the review ?
I am looking over the package. I am still compiling feedback.
Your quote is taken out of context. It actually says: > When a review ticket is assigned to a reviewer who does not respond to comments for one month, a comment is added to the ticket indicating that the review is stalled and that a response is needed soon. That means that after one month of inactivity, a comment should be added that the review is stalled. One week after **that** comment, the reviewer will get reset.
It would be great if we add this and replace p7zip with it.
This needs a license review because of all the public domain files https://gitlab.com/fedora/legal/fedora-license-data/-/issues/640
I put up https://bugzilla.redhat.com/show_bug.cgi?id=2361484 which I was working on earlier. It can be expanded later to provide all the compilation targets used for 7zip, but for now it produces 7zz and is parallel installable with the current p7zip
Since the legal stuff seems to be squaring away, here's the spec review: > %global _debugsource_template %{nil} This is bad. We don't want debuginfo packages disabled. This is a sign that something is going wrong in the spec file elsewhere. > %define stripped_version 2409 This should be turned into a something that is generated from %{version} so that it doesn't need to be updated in two places. > Source1: p7zip.sh > Source2: p7zip.1 > Source3: 7z.sh > Source4: 7z.1 > Source5: 7za.sh > Source6: 7za.1 > Source7: 7zr.sh > Source8: 7zr.1 > Source9: 7zz.sh > Source10: 7zz.1 > Patch0: fix-compatib-with-p7zip.patch Where are these from? There's no indication of where they came from. > BuildRequires: dos2unix > BuildRequires: gcc-c++ Missing "BuildRequires: make" Cf. https://fedoraproject.org/wiki/Changes/Remove_make_from_BuildRoot > Provides: p7zip = %{version} > Provides: p7zip-full = %{version} > Obsoletes: p7zip < %{version} > Obsoletes: p7zip-full < %{version} These need to use "%{version}-%{release}", and you're missing archful Provides (ie "Provides: p7zip%{?_isa} = %{version}-%{release}"). Also, these stanzas are completely wrong, since we never had p7zip-full. Please review how the upgrade path is supposed to work to match our actual p7zip package. > Provides: p7zip-plugins = %{version} > Obsoletes: p7zip-plugins < %{version} Also these need to use "%{version}-%{release}", and you're missing archful Provides. > Provides: p7zip-doc = %{version} > Obsoletes: p7zip-doc < %{version} Also these need to use "%{version}-%{release}". > #undefine _strict_symbol_defs_build > #set_build_flags > # Inject CFLAGS > sed -i 's#^ -fPIC# -fPIC %{optflags}#' CPP/7zip/7zip_gcc.mak > sed -i 's/LFLAGS_ALL = -s/LFLAGS_ALL =/' CPP/7zip/7zip_gcc.mak This doesn't seem to be working, as evidenced by you disabling the debuginfo subpackage template. > %ifarch x86_64 %ix86 %x86_64 > #sed -i 's/$(CXX) -o $(PROGPATH)/$(CXX) -Wl,-z,noexecstack -o $(PROGPATH)/' CPP/7zip/7zip_gcc.mak > %endif Why do you have a commented out command inside of conditional? This makes it effectively no-op.
Package legal review: > # CPP/7zip/Compress/LzfseDecoder.cpp is under the BSD-3-Clause > # C/Sha1.c and C/Sha256.c are in the public domain > License: BSD-3-Clause AND LGPL-2.1-or-later AND LicenseRef-Fedora-Public-Domain This is incomplete. Based on licensecheck results, "C/Xxh64.c" is BSD-2-Clause, so it is "BSD-2-Clause and BSD-3-Clause and LGPL-2.1-or-later and LicenseRef-Fedora-Public-Domain".
Based on earlier iterations flagged by the review service and reviewing the packaging, it seems like this package was copied from openSUSE, and indeed there seems very similar[1], though lacking the original attribution and changelog entries from the openSUSE package. Why did you feel the need to do this? The current proposed package seems to be quite broken for Fedora. [1]: https://build.opensuse.org/projects/openSUSE:Factory/packages/7zip/files/7zip.spec?expand=1
is based more on Debian than openSuse, I tried use the best of the 2 packages. We can't use uasm like opensuse use, so is Debian solution without asm I think Debian recently have progress and adopt another asm compiler IIRC > Source1: p7zip.sh > Source2: p7zip.1 > Source3: 7z.sh > Source4: 7z.1 > Source5: 7za.sh > Source6: 7za.1 > Source7: 7zr.sh > Source8: 7zr.1 > Source9: 7zz.sh > Source10: 7zz.1 > Patch0: fix-compatib-with-p7zip.patch the files are the man pages and wrappers copied from one of them
In that case, then you should endeavor to credit where they came from. That provenance matters, since we need to know how it is licensed and who to share fixes with if we change them.
The goal is keep compatibility with p7zip and , since February I'm using 7zip in my computer [1] (replacing p7zip) and I didn't saw any problem but I don't use it often . Also Arch Linux have and interesting approach with mason.build https://aur.archlinux.org/cgit/aur.git/tree/meson.build.template?h=7-zip [1] 7zip-24.09-1.fc41.x86_64 7zip-doc-24.09-1.fc41.noarch 7zip-plugins-24.09-1.fc41.x86_64
Spec URL: https://sergiomb.fedorapeople.org/7zip/7zip.spec SRPM URL: https://sergiomb.fedorapeople.org/7zip/7zip-24.09-1.fc43.src.rpm - clean source and remove Rar references applied with clean_7zip.sh - Added comments about origin of *.sh, *.1 and fix-compatib-with-p7zip.patch - Fixed provides I don't know how disable %global _debugsource_template %{nil} [mockbuild@434dbe07ee7648b8aecfcc4caddf7f5c 7zip-24.09]$ ll total 16 drwxr-xr-x 5 mockbuild mock 4096 Apr 24 01:05 Asm drwxr-xr-x 3 mockbuild mock 4096 Apr 24 01:05 C drwxr-xr-x 5 mockbuild mock 4096 Apr 24 01:05 CPP drwxr-xr-x 2 mockbuild mock 4096 Apr 24 01:14 DOC -rw-r--r-- 1 mockbuild mock 0 Apr 24 01:16 debugfiles.list -rw-r--r-- 1 mockbuild mock 0 Apr 24 01:16 debuglinks.list -rw-r--r-- 1 mockbuild mock 0 Apr 24 01:16 debugsourcefiles.list -rw-r--r-- 1 mockbuild mock 0 Apr 24 01:16 debugsources.list -rw-r--r-- 1 mockbuild mock 0 Apr 24 01:16 elfbins.list > This doesn't seem to be working, as evidenced by you disabling the debuginfo subpackage template. g++ -O2 -c -Werror -Wall -Wextra -Waddress -Waggressive-loop-optimizations -Wattributes -Wcast-align -Wcomment -Wdiv-by-zero -Wformat-contains-nul -Winit-self -Wint-to-pointer-cast -Wunused -Wunused-macros -Wbool-compare -Wduplicated-cond -Wbool-operation -Wconversion -Wdangling-else -Wduplicated-branches -Wimplicit-fallthrough=5 -Wint-in-bool-context -Wmaybe-uninitialized -Wmisleading-indentation -Wcast-align=strict -Wmissing-attributes -Waddress-of-packed-member -DNDEBUG -D_REENTRANT -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -fPIC -O2 -flto=auto -ffat-lto-objects -fexceptions -g -grecord-gcc-switches -pipe -Wall -Wno-complain-wrong-lang -Werror=format-security -Wp,-U_FORTIFY_SOURCE,-D_FORTIFY_SOURCE=3 -Wp,-D_GLIBCXX_ASSERTIONS -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -fstack-protector-strong -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -march=x86-64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -mtls-dialect=gnu2 -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -o b/g/UpdateCallbackConsole.o ../../UI/Console/UpdateCallbackConsole.cpp maybe is the flags here: g++ -o b/g/7zr -s -DNDEBUG -z noexecstack b/g/7zCrc.o b/g/7zCrcOpt.o b/g/7zStream.o b/g/Aes.o b/g/AesOpt.o b/g/Alloc.o b/g/Bcj2.o b/g/Bcj2Enc.o b/g/Bra.o b/g/Bra86.o b/g/BraIA64.o b/g/CpuArch.o b/g/Delta.o b/g/LzFind.o b/g/Lzma2Dec.o b/g/Lzma2DecMt.o b/g/Lzma2Enc.o b/g/LzmaDec.o b/g/LzmaEnc.o b/g/MtCoder.o b/g/MtDec.o b/g/Sha256.o b/g/Sha256Opt.o b/g/SwapBytes.o b/g/Xz.o b/g/XzDec.o b/g/XzEnc.o b/g/XzIn.o b/g/XzCrc64.o b/g/XzCrc64Opt.o b/g/LzFindMt.o b/g/LzFindOpt.o b/g/Threads.o b/g/StreamBinder.o b/g/VirtThread.o b/g/MyWindows.o b/g/CommandLineParser.o b/g/CRC.o b/g/CrcReg.o b/g/DynLimBuf.o b/g/IntToString.o b/g/ListFileUtils.o b/g/LzFindPrepare.o b/g/MyString.o b/g/MyVector.o b/g/NewHandler.o b/g/Sha256Prepare.o b/g/Sha256Reg.o b/g/StdInStream.o b/g/StdOutStream.o b/g/StringConvert.o b/g/StringToInt.o b/g/UTFConvert.o b/g/Wildcard.o b/g/XzCrc64Init.o b/g/XzCrc64Reg.o b/g/ErrorMsg.o b/g/FileDir.o b/g/FileFind.o b/g/FileIO.o b/g/FileLink.o b/g/FileName.o b/g/PropVariant.o b/g/PropVariantConv.o b/g/System.o b/g/SystemInfo.o b/g/TimeUtils.o b/g/Bcj2Coder.o b/g/Bcj2Register.o b/g/BcjCoder.o b/g/BcjRegister.o b/g/BranchMisc.o b/g/BranchRegister.o b/g/ByteSwap.o b/g/CopyCoder.o b/g/CopyRegister.o b/g/DeltaFilter.o b/g/Lzma2Decoder.o b/g/Lzma2Encoder.o b/g/Lzma2Register.o b/g/LzmaDecoder.o b/g/LzmaEncoder.o b/g/LzmaRegister.o b/g/XzDecoder.o b/g/XzEncoder.o b/g/7zAes.o b/g/7zAesRegister.o b/g/MyAes.o b/g/MyAesReg.o b/g/RandGen.o b/g/CreateCoder.o b/g/CWrappers.o b/g/FilePathAutoRename.o b/g/FileStreams.o b/g/InBuffer.o b/g/InOutTempBuffer.o b/g/FilterCoder.o b/g/LimitedStreams.o b/g/MethodId.o b/g/MethodProps.o b/g/MultiOutStream.o b/g/OffsetStream.o b/g/OutBuffer.o b/g/ProgressUtils.o b/g/PropId.o b/g/StreamObjects.o b/g/StreamUtils.o b/g/UniqBlocks.o b/g/LzmaHandler.o b/g/SplitHandler.o b/g/XzHandler.o b/g/CoderMixer2.o b/g/DummyOutStream.o b/g/HandlerOut.o b/g/InStreamWithCRC.o b/g/ItemNameUtils.o b/g/MultiStream.o b/g/OutStreamWithCRC.o b/g/ParseProperties.o b/g/7zCompressionMode.o b/g/7zDecode.o b/g/7zEncode.o b/g/7zExtract.o b/g/7zFolderInStream.o b/g/7zHandler.o b/g/7zHandlerOut.o b/g/7zHeader.o b/g/7zIn.o b/g/7zOut.o b/g/7zProperties.o b/g/7zRegister.o b/g/7zSpecStream.o b/g/7zUpdate.o b/g/ArchiveCommandLine.o b/g/ArchiveExtractCallback.o b/g/ArchiveOpenCallback.o b/g/Bench.o b/g/DefaultName.o b/g/EnumDirItems.o b/g/Extract.o b/g/ExtractingFilePath.o b/g/HashCalc.o b/g/LoadCodecs.o b/g/OpenArchive.o b/g/PropIDUtils.o b/g/SetProperties.o b/g/SortUtils.o b/g/TempFiles.o b/g/Update.o b/g/UpdateAction.o b/g/UpdateCallback.o b/g/UpdatePair.o b/g/UpdateProduce.o b/g/BenchCon.o b/g/ConsoleClose.o b/g/ExtractCallbackConsole.o b/g/HashCon.o b/g/List.o b/g/Main.o b/g/MainAr.o b/g/OpenCallbackConsole.o b/g/PercentPrinter.o b/g/UpdateCallbackConsole.o b/g/UserInputUtils.o -lpthread -ldl
Copr build: https://copr.fedorainfracloud.org/coprs/build/8957364 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2346041-7zip/fedora-rawhide-x86_64/08957364-7zip/fedora-review/review.txt Please take a look if any issues were found. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
[fedora-review-service-build] Enabled debuginfo
Copr build: https://copr.fedorainfracloud.org/coprs/build/8957451 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2346041-7zip/fedora-rawhide-x86_64/08957451-7zip/fedora-review/review.txt Please take a look if any issues were found. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
[fedora-review-service-build] (In reply to Michel Lind from comment #18) > I put up https://bugzilla.redhat.com/show_bug.cgi?id=2361484 which I was > working on earlier. It can be expanded later to provide all the compilation > targets used for 7zip, but for now it produces 7zz and is parallel > installable with the current p7zip I copied license and description for your spec , I also verified that we can't use LOCAL_FLAGS="%{optflags}" , it will remove definitions from some builds, specifically -DZ7_EXTRACT_ONLY -DZ7_NO_READ_FROM_CODER -DZ7_SFX from CPP/7zip/Bundles/SFXCon [1] We have a bug opened since 2021 bug#1946988 , from this bug, we can see that 7zip could only recently can be compiled on Linux it had some forks and others problems . You are welcome to maintain the package , with inspiration on your work I think I achieve even a better way to inject CFLAGS, IMO the package is ready be to imported Best regards, [1] make: Leaving directory '/builddir/build/BUILD/7zip-24.09-build/7zip-24.09/CPP/7zip/Bundles/SFXCon' ../../Bundles/SFXCon/SfxCon.cpp: In function ‘int Main2(int, char**)’: ../../Bundles/SFXCon/SfxCon.cpp:461:11: error: cannot convert ‘UString’ to ‘IHashCalc*’ 461 | errorMessage, stat); | ^~~~~~~~~~~~ | | | UString
Created attachment 2087235 [details] The .spec file difference from Copr build 8957451 to 8967421
Copr build: https://copr.fedorainfracloud.org/coprs/build/8967421 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2346041-7zip/fedora-rawhide-x86_64/08967421-7zip/fedora-review/review.txt Please take a look if any issues were found. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
Created attachment 2087241 [details] The .spec file difference from Copr build 8967421 to 8967644
Copr build: https://copr.fedorainfracloud.org/coprs/build/8967644 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2346041-7zip/fedora-rawhide-x86_64/08967644-7zip/fedora-review/review.txt Please take a look if any issues were found. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
@ngompa13 , can we finish this package review
Have you sorted things out with Michel and his packaging yet?
I replied to him in comment #29 I copied license and description for his spec , I also verified that we can't use his LOCAL_FLAGS="%{optflags}" , it will remove definitions from some builds, specifically -DZ7_EXTRACT_ONLY -DZ7_NO_READ_FROM_CODER -DZ7_SFX from CPP/7zip/Bundles/SFXCon , what's more to sort out ? - clean source and remove Rar references applied with clean_7zip.sh - Added comments about origin of *.sh, *.1 and fix-compatib-with-p7zip.patch - Fixed provides - Enabled debug. I achieved even a better way to inject CFLAGS I think all is fixed since 2025-04-25 and package is ready and Michel is welcome to maintain the package
(In reply to Sergio Basto from comment #37) > I replied to him in comment #29 > > I copied license and description for his spec , I also verified that we > can't use his LOCAL_FLAGS="%{optflags}" , it will remove definitions from > some builds, specifically -DZ7_EXTRACT_ONLY -DZ7_NO_READ_FROM_CODER > -DZ7_SFX from CPP/7zip/Bundles/SFXCon , what's more to sort out ? > you can always add to LOCAL_FLAGS, see my 7z build %make_build %make_opts LOCAL_FLAGS="%{optflags} -DZ7_EXTERNAL_CODECS" -C CPP/7zip/UI/Console/ > > - clean source and remove Rar references applied with clean_7zip.sh > - Added comments about origin of *.sh, *.1 and fix-compatib-with-p7zip.patch > - Fixed provides > - Enabled debug. I achieved even a better way to inject CFLAGS > > I think all is fixed since 2025-04-25 and package is ready and Michel is > welcome to maintain the package I'll let the reviewers figure out what to do, I think personally starting with a spec that has no borrowing from any other distribution is probably better. But either way the only reason I'm even proposing a spec is because we need 7zip available one way or another (since branching the really old p7zip for EPEL 10 is obviously not a good solution)
I recognize that this review was posted before the one in bug 2361484. Normally I would stick with the original review and close subsequent ones as duplicates, but in this case Michel's spec file is in much better shape than this one. I agree with the previously stated points about proper attribution for things borrowed from other distributions. In particular the man pages should be sent to the upstream project, no just cargo culted around. Since you're already copying in sections of his spec file, I think the best path forward is to just approve his review, and then he can add you as a co-maintainer after it's imported to distgit. For that reason, I'm closing this review as a duplicate of Michel's. I hope this is an acceptable compromise to you to move this package forward. *** This bug has been marked as a duplicate of bug 2361484 ***
Hi, 1. LOCAL_FLAGS is use some cases and not in other cases , I don't think is a good place to set it duplicated switches 2. the spec is original form the fedora p7zip.spec it preserve same locations , it provides a patch to be compatible with p7zip, and I copied things from Debian and from Suse , I checked Arch, I copied the good things from there, , it took me same time figure out the solution was dropped asm build like Debian because uasm is not a good license. 3. I build 6 binaries not one , I think we need keep 7zip-plugins , I maintain the structure of old package and compatibility, we can replace p7zip with this 7zip 4. my gcc lines are without any duplicated switch