Fix cleanup of temporary files

[pabs3]

No description provided.

[spacewander]

What about building the script content in a subshell and redirecting it directly to the destination? This avoids mktemp, chmod on temp files from the beginning.

[pabs3]

Maybe something like this?

• source the helper/ scripts rather than embedding them
  • add the source command to each script in bin/
• install the helper/ scripts in /usr/share/git-extras/helper or similar
• separate out a build target from the install target
• build each of the to-be-installed scripts in a pattern rule that would just add the install path for the helper/ scripts.

[spacewander]

Is this plan a little complicated? We can just use the subshell's stdout as a temp file, so no need to use a real file.

[hyperupcall]

@spacewander Could we keep using the temporary file? Things kind of look complicated enough with this being embedded in a Makefile and all,.

[spacewander]

@hyperupcall
I am OK to keep the temporary file here, since you think removing it would require complicated code.

[pabs3]

Is this plan a little complicated?

The plan I proposed is the normal way most projects get built from source, the current setup for git-extras seemed more complicated to me. I don't mind if you want to keep the current setup though.

[hyperupcall]

Looks great so far! Was a little bothersome to learn that git-extras didn't cleanup properly after execution.

Just had a few minor comments; once addressed, I'll approve!

[hyperupcall]

@spacewander Could we keep using the temporary file? Things kind of look complicated enough with this being embedded in a Makefile and all,.

[hyperupcall]

Could this be moved out to a separate PR? For matching the argument, would prefer using $1 == @(--proceed|-p) (enabling extglob) or keeping the existing structure and using group commands ({ true && true; }) instead of subshells.

[pabs3]

This commit used to be on the old master branch before it got renamed to main. I based my branch on master because I didn't notice the new main branch, oops! The commit was already merged into main as commit 52897f7, albeit slightly modified. If I can get permission to rebase onto main then I can remove this commit and resolve this problem.

[hyperupcall]

Definitely, go ahead

[pabs3]

I wonder if git-extras needs something more comprehensive to prevent future instances of this issue? Maybe git_extra_mktemp could add the files to a global array of files to be cleaned up, and then a trap _git_extras_exit INT TERM EXIT could call any commands registered as exit traps and then delete all the temporary files.

[spacewander]

I wonder if git-extras needs something more comprehensive to prevent future instances of this issue? Maybe git_extra_mktemp could add the files to a global array of files to be cleaned up, and then a trap _git_extras_exit INT TERM EXIT could call any commands registered as exit traps and then delete all the temporary files.

Personally, I think temporary files are OK because most git-* commands generate small text files. I am worried that adding a temporary files list may make the code complicated.