coop-deluxe / sm64coopdx

An official continuation of https://github.com/djoslin0/sm64ex-coop on sm64coopdx for the enhancements and progress it already has.
https://sm64coopdx.com
322 stars 56 forks source link

Make patches work when using git submodules #285

Closed Gumbo64 closed 1 week ago

Gumbo64 commented 2 weeks ago

create_patches.sh checks if ".git" is a folder, but when using git submodules, ".git" is a file. Otherwise though patches work perfectly with git submodules

robertkirkman commented 2 weeks ago

My suggested revision:

--- a/tools/create_patch.sh
+++ b/tools/create_patch.sh
@@ -11,11 +11,7 @@ then
 fi

 # Make sure this is a git repository
-if [ ! -d .git ]
-then
-    echo 'Error: The current directory is not a Git repository.'
-    exit 1
-fi
+git status > /dev/null || exit 1

 # 'git diff' is stupid and doesn't show new untracked files, so we must add them first.
 git add .

Reasoning: error handling logic for this task already exists inside of the git program. Activating it can check for this error reliably using the logic the creators of the git program wrote for this purpose.

Example output in the success state:

$ tools/create_patch.sh test1.patch
Unstaged changes after reset:
M   Makefile
M   extract_assets.py
D   sound/samples_assets.c
D   sound/sequences_assets.c
M   sound/sound_data.c
M   src/audio/load.c
M   tools/Makefile
M   tools/assemble_sound.py
M   tools/create_patch.sh
M   tools/sdk-tools/adpcm/vadpcm_enc.c
M   tools/sdk-tools/adpcm/vpredictor.c

Example output in the failure state:

$ tools/create_patch.sh test1.patch
fatal: not a git repository (or any of the parent directories): .git
robertkirkman commented 2 weeks ago

Another example change that behaves exactly the same as above, but with fewer lines of code; the intent might not be as clear from this one though.

--- a/tools/create_patch.sh
+++ b/tools/create_patch.sh
@@ -10,15 +10,8 @@ then
     exit 1
 fi

-# Make sure this is a git repository
-if [ ! -d .git ]
-then
-    echo 'Error: The current directory is not a Git repository.'
-    exit 1
-fi
-
 # 'git diff' is stupid and doesn't show new untracked files, so we must add them first.
-git add .
+git add . || exit 1
 # Generate the patch.
 git diff -p --staged > "$1"
 # Undo the 'git add'. 
Gumbo64 commented 2 weeks ago

Good revisions, I tested both of them and they work well. I think your second one is better because the first one seems to list changes about the the upper repository, which may be misleading. The second one also seems to pass the error code up to the makefile when the first one didn't. This is just in my particular setup though of course. Good job, I will add it to the pull request. image

robertkirkman commented 2 weeks ago

Hm, that is strange, for me the return code in a minimal test is 1.

~ $ cat scripttest.sh 
#!/bin/bash
git status > /dev/null || exit 1
~ $ ./scripttest.sh 
fatal: not a git repository (or any of the parent directories): .git
~ $ echo $?
1
~ $ 

If you want to show your sm64-AI-DX/Makefile code then I might be able to tell you why that happens, but if you don't have that public then I guess you can stick with the 2nd option if it's working