Page MenuHomeSolus

Initial inclusion of Broot, resolves T8840
Needs ReviewPublic

Authored by sethfl on Tue, Mar 24, 1:07 PM.

Details

Reviewers
DataDrake
JoshStrobl
Group Reviewers
Triage Team
Maniphest Tasks
T8840: Broot
Summary

Broot is a new way to see and navigate directory trees using cd, tree, and fuzzy search. Resolves T8840

Changelog: Can be found here

Test Plan
  • Build Package
  • Install Package
  • Run "Broot" in multiple directories
  • Use all functions such as opening files, search for files, and opening other directories

Diff Detail

Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

sethfl created this revision.Tue, Mar 24, 1:07 PM
sethfl requested review of this revision.Tue, Mar 24, 1:07 PM
algent added a subscriber: algent.Tue, Mar 24, 1:47 PM

You can add to "Summary": Resolves T8840
arc diff --edit to do that

This will link the "broot" task with this diff.

sethfl updated this revision to Diff 20413.Tue, Mar 24, 3:41 PM
sethfl edited the summary of this revision. (Show Details)
sethfl added a task: T8840: Broot.

add resolution to summary

Is there a reason for using a binary from a third-party repo instead of building it ourselves?

Jacalz added a subscriber: Jacalz.Tue, Mar 24, 4:18 PM

Regarding what @EbonJaeger said, I think it would be much better to use the actual tarball releases over at https://github.com/Canop/broot/releases and build it ourselves. I don't think the current implementation will be accepted unless there is a very specific reason and especially not when the binaries come from someone else than the project owner...

sethfl added a comment.EditedTue, Mar 24, 4:24 PM

@EbonJaeger @Jacalz The repository is actually mine on github. The binary comes from official site here. All I did was create a makefile for the binary and create a repo for both on github in order to make it easier to package for multiple distros. If that is against any rules though, I was unaware, and I will work on a .yml based off the upstream tarball until we get word from the devs.

@EbonJaeger @Jacalz The repository is actually mine on github. The binary comes from official site here. All I did was create a makefile for the binary and create a repo for both on github in order to make it easier to package for multiple distros. If that is against any rules though, I was unaware, and I will work on a .yml based off the upstream tarball until we get word from the devs.

I am pretty sure it is. I think it more or less is due to security reasons. It is very hard to know if you tampered with the source or not. Having it built from official source code on the build server makes sure that it doesn't contain any nasty surprises.

@EbonJaeger @Jacalz The repository is actually mine on github. The binary comes from official site here. All I did was create a makefile for the binary and create a repo for both on github in order to make it easier to package for multiple distros. If that is against any rules though, I was unaware, and I will work on a .yml based off the upstream tarball until we get word from the devs.

We always build from source if source is available and there are no extenuating circumstance. I won't accept this unless you are building from these source: https://github.com/Canop/broot

DataDrake requested changes to this revision.Tue, Mar 24, 4:40 PM
This revision now requires changes to proceed.Tue, Mar 24, 4:40 PM

Okay, I completely understand. I'm working on a new .yml build with rust now :)

sethfl updated this revision to Diff 20416.Tue, Mar 24, 5:45 PM

change source from binary to tarball

@DataDrake updated the .yml to have the tarball as the source

DataDrake requested changes to this revision.Tue, Mar 24, 5:49 PM
DataDrake added inline comments.
package.yml
11

Should be a list format, even for one item.

14

Did you mean "setup"? "prep" doesn't do anything.

20

Should just be:

install -Dm00755 target/release/broot $installdir/usr/bin/broot
21

We don't ship license files unless upstream does it of their own volition.

This revision now requires changes to proceed.Tue, Mar 24, 5:49 PM
sethfl updated this revision to Diff 20418.Wed, Mar 25, 1:41 AM

Revised .yml according to DataDrake's comments

sethfl updated this revision to Diff 20565.Wed, Apr 1, 4:00 PM
sethfl marked 4 inline comments as done.

update to new upstream relese

JoshStrobl requested changes to this revision.Thu, Apr 2, 6:09 AM
JoshStrobl added a subscriber: JoshStrobl.
JoshStrobl added inline comments.
package.yml
13

description should go after summary, not after builddeps.

18

According to https://github.com/Canop/broot/blob/master/build.rs they are also building shell completions and I see a man page dir in the project's root. We should ideally be installing what shell completions they do have:

  • bash: /usr/share/bash-completion/completions/
  • fish: /usr/share/fish/completions/
  • zsh: /usr/share/zsh/site-functions/

manpage location will be determined by the section they are expecting, probably 1 for general commands, so it should be installed to /usr/share/man/man1/ and be sure it has .1 appended to the end of the man page file name.

This revision now requires changes to proceed.Thu, Apr 2, 6:09 AM
sethfl updated this revision to Diff 20581.Thu, Apr 2, 2:03 PM

added shell completions

sethfl updated this revision to Diff 20582.Thu, Apr 2, 2:22 PM
sethfl marked an inline comment as done.

forgot to add manpages :/