glutanimate / PDFMtEd

View and modify PDF metadata on Linux graphically
GNU General Public License v3.0
191 stars 22 forks source link

Create BinPath, LauncherPath and IconPath if they don't exist. #19

Open ifohancroft opened 3 years ago

ifohancroft commented 3 years ago

Changed /usr/local/ to /usr/ to avoid the multiple problems that come from using /usr/local/ Also, fixes #9

ifohancroft commented 3 years ago

Just wanted to see if you have had the time to check this PR?

glutanimate commented 3 years ago

Hey Ifo, Thanks for the ping (plus your original contribution of course), and sorry for taking ages to look at this!

So in general, I wouldn't be completely opposed to changing the root installation path to /usr, but I do feel like this goes against the usual conventions with manually installed projects.

Naively speaking and without having looked at PDFMtEd in a while, how about creating BinPath, LauncherPath, and IconPath recursively if they do not exist? That should be possible with mkdir -p.

ifohancroft commented 3 years ago

Hey, Most welcome!

That's fine, I am just glad that you are well and here now.

That's a very good point. I keep forgetting about that.

I have an idea about it but I will comment later tonight about it on here, as I have something to do first and I need to quickly research something about it.

ifohancroft commented 3 years ago

I wanted to check if there would be an error from mkdir -p if the directories all exist and perhaps add a check if there is an error, but there isn't one, so I just changed /usr to /usr/local again and added:

mkdir -p "$BinPath"
mkdir -p "$LaunchPath"
mkdir -p "$IconPath"

To install.sh. Although, perhaps I should perhaps rename the PR.

nbehrnd commented 2 years ago

Ping @glutanimate @ifohancroft Are there updates regarding path /usr/local/share/icons/hicolor/scalable/apps? Speaking to Debian 12/bookworm (branch testing), I equally had to apply the «the trick» from here for a pristine installation of the program.

ifohancroft commented 2 years ago

Ping @glutanimate @ifohancroft Are there updates regarding path /usr/local/share/icons/hicolor/scalable/apps? Speaking to Debian 12/bookworm (branch testing), I equally had to apply the «the trick» from here for a pristine installation of the program.

@nbehrnd I am basically waiting for @glutanimate to merge this PR. IIRC this creates the directories if they don't exist.

ifohancroft commented 2 years ago

So in general, I wouldn't be completely opposed to changing the root installation path to /usr, but I do feel like this goes against the usual conventions with manually installed projects.

Apparently this seems to be the exception as opposed to the standard in the Filesystem Hierarchy Standard. If you take a look here where it lists deviations from the standard, it lists how FreeBSD uses /usr and /usr/local to differentiate between custom software and software from the distribution. So I guess creating the directories in /usr won't be going against the standard, although it does seem more common for software to install itself under /usr/local/

aliraeisdanaei commented 1 year ago

I wanted to check if there would be an error from mkdir -p if the directories all exist and perhaps add a check if there is an error, but there isn't one, so I just changed /usr to /usr/local again and added:

mkdir -p "$BinPath"
mkdir -p "$LaunchPath"
mkdir -p "$IconPath"

To install.sh. Although, perhaps I should perhaps rename the PR.

Why doesn't the script first try the /usr/local/ then if it doesn't exist go to /usr/

Because this solution seems to create unnecessary directories.

aliraeisdanaei commented 1 year ago

I wanted to check if there would be an error from mkdir -p if the directories all exist and perhaps add a check if there is an error, but there isn't one, so I just changed /usr to /usr/local again and added:

mkdir -p "$BinPath"
mkdir -p "$LaunchPath"
mkdir -p "$IconPath"

To install.sh. Although, perhaps I should perhaps rename the PR.

Why doesn't the script first try the /usr/local/ then if it doesn't exist go to /usr/

Because this solution seems to create unnecessary directories.

I created a pull request for this as can be seen here