search

facebook / PathPicker

PathPicker accepts a wide range of input -- output from git commands, grep results, searches -- pretty much anything. After parsing the input, PathPicker presents you with a nice UI to select which files you're interested in. After that you can open them in your favorite editor or execute arbitrary commands.
https://facebook.github.io/PathPicker/
MIT License
5.13k stars 282 forks source link

Bugs in debian/package.sh #317

Open TimSpence opened 4 years ago

TimSpence commented 4 years ago

Some have been previously mentioned: https://github.com/facebook/PathPicker/issues/256. The script makes assumptions about system capabilities and fails to terminate when those assumptions aren't met. In particular, it assumes fakeroot is available. If fakeroot is not available, execution continues and multiple files are deleted from the working copy. This is the outcome:

❯ git status
On branch fix-builder
Your branch is up to date with 'origin/master'.

Changes not staged for commit:
  (use "git add/rm <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
        modified:   DEBIAN/control
        deleted:    package.sh
        deleted:    usr/share/doc/pathpicker/changelog

It also assumes the script executes from debian/, so it fails when run from project root.

I can fix this in two ways.

  1. Fix package.sh so that it terminates if any command returns a non-zero exit code.
  2. Dockerize the build process so that the end user can build and install fpp without having to understand the package build process.

Option 1 is simpler, but 2 is preferable (as long as we assume the user has Docker running). Option 2 also automates the build process, which may be desirable if Facebook wants to host the package in a mirror.

bduffany commented 3 years ago

It also assumes "python" is available which isn't true on Ubuntu 20.04 AFAIK. It depends on either an installation of python 2 or installing the package python-is-python3 (which symlinks python to python3)

pcottle commented 3 years ago

Sorry about the delay here! Yes there are definitely a lot of assumptions in the build process -- having a dockerized version would be great but lets start by merging your PR so the script doesn't continue after errors.

Sorry for the delay in merging!