Bug 1487430

Summary: Review Request: git-lfs - Git extension for versioning large files
Product: [Fedora] Fedora Reporter: Elliott Sales de Andrade <quantum.analyst>
Component: Package ReviewAssignee: Neal Gompa <ngompa13>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: ignatenko, ngompa13, package-review
Target Milestone: ---Flags: ngompa13: fedora-review+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2017-11-18 21:37:54 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On: 1486510, 1486511    
Bug Blocks:    

Description Elliott Sales de Andrade 2017-08-31 22:04:08 UTC
Spec URL: https://copr-be.cloud.fedoraproject.org/results/qulogic/git-lfs/fedora-rawhide-x86_64/00596933-git-lfs/git-lfs.spec
SRPM URL: https://copr-be.cloud.fedoraproject.org/results/qulogic/git-lfs/fedora-rawhide-x86_64/00596933-git-lfs/git-lfs-2.2.1-2.fc28.src.rpm
Description: Git Large File Storage (LFS) replaces large files such as audio samples, videos, datasets, and graphics with text pointers inside Git, while storing the file contents on a remote server.
Fedora Account System Username: qulogic

Comment 1 Elliott Sales de Andrade 2017-08-31 22:07:22 UTC
*** Bug 1336168 has been marked as a duplicate of this bug. ***

Comment 2 Neal Gompa 2017-09-01 04:30:58 UTC
Taking this review.

Comment 3 Neal Gompa 2017-09-01 04:47:12 UTC
Initial pass through:

> # Generate devel rpm
> %global with_devel 0

Why is this disabled? Is it not possible to use Git LFS code as a module in other Go programs?

> # Generate unit-test rpm
> %global with_unit_test 0

Why is this disabled?

> %post
> /bin/git-lfs install --system

Please use "%{_bindir}/%{name}" here, as that's how it is actually installed.

> %preun
> if [ $1 -gt 0 ]; then
> /bin/git-lfs uninstall
> fi
> exit 0

Same here, and also properly indent the shell script here.

> %files
> %license LICENSE.md
> %{_bindir}/%{name}
> %doc %{_mandir}/man1/%{name}*.1*
> %doc %{_mandir}/man5/%{name}*.5*

It is redundant to declare man pages as documentation, as rpm auto-marks files installed into %{_mandir}/*/* as documentation files.

Comment 4 Elliott Sales de Andrade 2017-09-01 04:56:36 UTC
I can't say for certain whether you can or cannot use it as a module, but the draft Go spec doesn't even try to build devel/unit-test RPMs: https://fedoraproject.org/wiki/PackagingDrafts/Go#Packaging_a_binary
Instead, I left those in there in case we need to enable them later.

Comment 5 Elliott Sales de Andrade 2017-09-01 05:24:23 UTC
According to godoc, there are no importers: https://godoc.org/github.com/git-lfs/git-lfs

Comment 6 Elliott Sales de Andrade 2017-09-01 08:37:47 UTC
Fixed the initial comments.

Spec URL: https://copr-be.cloud.fedoraproject.org/results/qulogic/git-lfs/fedora-rawhide-x86_64/00597054-git-lfs/git-lfs.spec
SRPM URL: https://copr-be.cloud.fedoraproject.org/results/qulogic/git-lfs/fedora-rawhide-x86_64/00597054-git-lfs/git-lfs-2.2.1-3.fc28.src.rpm
Description: Git Large File Storage (LFS) replaces large files such as audio samples, videos, datasets, and graphics with text pointers inside Git, while storing the file contents on a remote server.
Fedora Account System Username: qulogic

Comment 7 Neal Gompa 2017-09-01 17:14:53 UTC
Package was generated through gofed, simplifying the review considerably.

- Conforms to packaging guidelines (gofed generated spec)
- license correct and valid
- license file installed correctly
- Binaries for applications installed
- Scriptlets look sane

PACKAGE APPROVED

Comment 8 Igor Gnatenko 2017-09-01 17:21:48 UTC
to me scriptlets don't look sane... I would expect it to be executed while package is built, but this is one thing...

Another thing could be done is to reference all files it creates via %ghost.

Comment 9 Elliott Sales de Andrade 2017-09-01 20:24:16 UTC
According to its man page, git only seems to support a single /etc/gitconfig file and not a directory like /etc/mercurial/hgrc.d. So I don't see how this could be done at build time. The file is not technically owned by git-lfs.

Comment 10 Gwyn Ciesla 2017-09-01 21:26:20 UTC
(fedrepo-req-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/git-lfs

Comment 11 Neal Gompa 2017-09-02 13:22:17 UTC
(In reply to Igor Gnatenko from comment #8)
> to me scriptlets don't look sane... I would expect it to be executed while
> package is built, but this is one thing...
> 
> Another thing could be done is to reference all files it creates via %ghost.

Unfortunately, git is insane, so... :(

Comment 12 Fedora Update System 2017-09-03 02:15:30 UTC
git-lfs-2.2.1-3.fc26 has been submitted as an update to Fedora 26. https://bodhi.fedoraproject.org/updates/FEDORA-2017-090bbadbe3

Comment 13 Fedora Update System 2017-09-03 02:15:53 UTC
git-lfs-2.2.1-3.fc27 has been submitted as an update to Fedora 27. https://bodhi.fedoraproject.org/updates/FEDORA-2017-6ae6d44aaa

Comment 14 Fedora Update System 2017-09-03 19:55:58 UTC
git-lfs-2.2.1-3.fc27 has been pushed to the Fedora 27 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2017-6ae6d44aaa

Comment 15 Fedora Update System 2017-09-04 06:51:36 UTC
git-lfs-2.2.1-3.fc26 has been pushed to the Fedora 26 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2017-090bbadbe3

Comment 16 Fedora Update System 2017-09-12 00:23:00 UTC
git-lfs-2.2.1-3.fc26 has been pushed to the Fedora 26 stable repository. If problems still persist, please make note of it in this bug report.

Comment 17 Fedora Update System 2017-11-04 05:08:37 UTC
git-lfs-2.3.4-2.fc27 has been submitted as an update to Fedora 27. https://bodhi.fedoraproject.org/updates/FEDORA-2017-6ae6d44aaa

Comment 18 Fedora Update System 2017-11-04 19:04:17 UTC
git-lfs-2.3.4-2.fc27 has been pushed to the Fedora 27 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2017-6ae6d44aaa

Comment 19 Fedora Update System 2017-11-14 15:32:28 UTC
git-lfs-2.3.4-2.fc27 has been pushed to the Fedora 27 stable repository. If problems still persist, please make note of it in this bug report.

Comment 20 Elliott Sales de Andrade 2017-11-18 21:37:54 UTC
Only reopened because the freeze took so long.