[Bug 562977] Review Request: vifm - Lightweight file manager with vi like keybindings

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=562977

Mamoru Tasaka <mtasaka at ioa.s.u-tokyo.ac.jp> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mtasaka at ioa.s.u-tokyo.ac.jp

--- Comment #2 from Mamoru Tasaka <mtasaka at ioa.s.u-tokyo.ac.jp> 2010-04-11 14:39:39 EDT ---
Some notes:

! BuildRoot
  - BuildRoot is no longer needed for Fedora (rpmlint may complain, however
    it can be ignored)
    https://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag

* Timestamps
  - Please consider to use
----------------------------------------------------------------------------------
make install DESTDIR=$RPM_BUILD_ROOT INSTALL="install -p"
----------------------------------------------------------------------------------
    to keep timestamps on installed files as much as possible.
    This method usually works for Makefiles generated by recent
    autotools.

! Documents
  ! Usually INSTALL file is for people trying to compile and install
    software by themselves and is not needed for people using rpm,
    however for this package I will leave it to how you think whether
    INSTALL file should be packaged or not.

* Directory ownership issue
  https://fedoraproject.org/wiki/Packaging/UnownedDirectories#Common_Mistakes
  - The directory %{_datadir}/%{name} itself is not owned by any packages.

* Source codes themselves
  - Some codes hardcode /usr/local for installed paths.
    ! For example from config.c:
---------------------------------------------------------------------------------
    19  #define CP_HELP "cp /usr/local/share/vifm/vifm-help.txt ~/.vifm"
    20  #define CP_RC "cp /usr/local/share/vifm/vifmrc ~/.vifm"
---------------------------------------------------------------------------------
      With this, launching vifm causes warnings like:
---------------------------------------------------------------------------------
 env LANG=C vifm
cp /usr/local/share/vifm/vifm-help.txt ~/.vifmcp: cannot stat
`/usr/local/share/vifm/vifm-help.txt': No such file or directory
Unable to find configuration file.
 Using defaults.cp: cannot stat `/usr/local/share/vifm/vifmrc': No such file or
directory
---------------------------------------------------------------------------------
     This needs fixing.

  - config.c reads:
---------------------------------------------------------------------------------
   144                  if(chdir(cfg.config_dir))
   145                  {
   146                          if(mkdir(cfg.config_dir, 0777))
   147                                  return;
   148                          if(mkdir(cfg.trash_dir, 0777))
   149                                  return;
   150                          if((f = fopen(help_file, "r")) == NULL)
   151                                  create_help_file();
   152                          if((f = fopen(rc_file, "r")) == NULL)
   153                                  create_rc_file();
   154                  }
---------------------------------------------------------------------------------
    Here "0700" is much safer.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.