Bug 1487430 - Review Request: git-lfs - Git extension for versioning large files
Summary: Review Request: git-lfs - Git extension for versioning large files
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Neal Gompa
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
: 1336168 (view as bug list)
Depends On: 1486510 1486511
Blocks:
TreeView+ depends on / blocked
 
Reported: 2017-08-31 22:04 UTC by Elliott Sales de Andrade
Modified: 2020-01-08 04:50 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2017-11-18 21:37:54 UTC
Type: ---
Embargoed:
ngompa13: fedora-review+


Attachments (Terms of Use)

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.


Note You need to log in before you can comment on or make changes to this bug.