Page MenuHomeSolus

Initial inclusion of Broot, resolves T8840
ClosedPublic

Authored by sethfl on Mar 24 2020, 1:07 PM.
Tags
None
Referenced Files
F11048423: D8534.id20416.diff
Thu, Aug 10, 4:35 PM
F11048422: D8534.id.diff
Thu, Aug 10, 4:35 PM
F11048421: D8534.id20408.diff
Thu, Aug 10, 4:35 PM
F11048420: D8534.id20595.diff
Thu, Aug 10, 4:35 PM
F11048418: D8534.id20581.diff
Thu, Aug 10, 4:35 PM
F11048417: D8534.id20418.diff
Thu, Aug 10, 4:35 PM
F11048416: D8534.id20413.diff
Thu, Aug 10, 4:35 PM
F11048415: D8534.id21193.diff
Thu, Aug 10, 4:35 PM

Details

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 Lint Coverage
Unit
No Test Coverage

Event Timeline

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

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

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?

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...

@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

This revision now requires changes to proceed.Mar 24 2020, 4:40 PM

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

change source from binary to tarball

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

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.Mar 24 2020, 5:49 PM

Revised .yml according to DataDrake's comments

sethfl marked 4 inline comments as done.

update to new upstream relese

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.Apr 2 2020, 6:09 AM
sethfl marked an inline comment as done.

forgot to add manpages :/

JoshStrobl added inline comments.
package.yml
19

Seems like a fair bit of redundancy with how you're doing completions installation. You could more easily accomplish all of this using a pushd into the target out directory and loop a list of the completions or the directories and install from there. Otherwise it's a good start.

This revision now requires changes to proceed.Apr 3 2020, 6:17 AM

I'll be honest, I don't really know how to use pushd commands, and even after reading manpages and articles I couldn't really understand it. I cut the install commands by half though by using the -t operand. Hope it works

thanks @algent, will update the diff soon :)

update to latest upstream release

There is still something to do.
move description from here:

builddeps  : 
    cargo
description: |
    A new way to see and navigate directory trees using cd, tree, and fuzzy search.
build      : |
    cargo build --release --locked

to here

summary    : A new way to see and navigate directory trees
description: |
    A new way to see and navigate directory trees using cd, tree, and fuzzy search.
networking : yes

move description to correct place. thanks @algent.

This revision was not accepted when it landed; it landed in state Needs Review.May 13 2020, 3:22 PM
This revision was automatically updated to reflect the committed changes.