auto-patcher icon indicating copy to clipboard operation
auto-patcher copied to clipboard

Automatically call batch.sh commands when files re missing (mateor/auto-patcher#74)

Open xee5ch opened this issue 11 years ago • 4 comments

This is meant to fix mateor/auto-patcher#74 by doing the following:

  1. Move the batch.sh commands into a function in auto_patcher, prep_patches.
  2. In batch.sh, remove duplicating expath and abspath functions and just source auto_patcher directly.
  3. Make batch.sh just call the prep_patches function from the source auto_patcher file.

xee5ch avatar Jun 09 '13 10:06 xee5ch

Okay, I think I need to hear a little more about your thinking here...I do understand the desire to remove duplicate code (very much) but I think if the cost is an if wrapper around the entire script, we might be better off with the duplicate code at this point.

Do you think we can find a way around that wrapper? From an end-user point of view (basically my philosophy is that the ease of use and error-catching above everything else) I think that adding batch.sh if the .tgz files are not found is very beneficial...but I think we might gone overboard on how this is implemented.

Am I following your thinking right? Feel free to convince me otherwise, I am very open to discussion.

Also, maybe there is an environmental issue? This is my output from the test I ran...http://pastebin.com/Mw8Hruze

The top of that paste is a diff showing the changes of yours I have, in case I missed one. At the end is the commands I ran.

mateor avatar Jun 18 '13 19:06 mateor

Well, the reason I wanted to implement this a certain was is my experience from Python, as it makes testing scripts, specifically those that will be library components much easier.

I even specifically looked for the if __name__ == "__main__" convention when Googling for this reason. What happens is that you can load all code in the file into another script later, but everything beneath will be executed if and only if executing the script in question. This had to be done if I take the code de-duplication route, so that if I source the auto_patcher script from batch.sh without that BASH_SOURCE block, it will start executing auto_patcher and complain how I did not give it the proper parameters, namely a ROM.

To take this further, the second reason would be for unit testing ideas you had. Unless you want to keep the unit tests in the auto_patcher file itself, it will be difficult to separate different logical units (like you have down with batch.sh and auto_patcher) and share functionality, haha dare I say functions themselves, without this method. I think the cleanliness makes it easier to code as stuff grows, but you are the boss here and this is my first FOSS adventure. So you tell me what you like.

As for your error, a quick glance makes it look like, yes, you got it right. I am not sure why the patch detection fails then. As I always try patching a ROM to test before handing off changes to you. It sounds like you do the same. Not sure what to do about that. I am really busy this week, so maybe I will take a look on the weekend if you do not mind?

xee5ch avatar Jun 19 '13 07:06 xee5ch

Okay, I did a little reading on that and see more what you were thinking. I know the python syntax, but have written only very basic scripts with it. I had to Google the convention!

I just (hopefully) finished the auto-updating feature that I accidentally spammed our history with. I added a .config file to the autopatcher so that people like you and I could turn off auto-updating, as it won't play very nicely with edits to our local script or branches. I added a flag for PRODUCTION with the test suite in mind. Do you think we could use some sort of flag there? Ideally, it could be set by the command line as well, if we have to manually edit the .config file to run tests, then I think the wrapper is better.

I am closer to merging it now than I was the other day, I have a better idea of what was trying to be accomplished. Let's think on it some more as batch.sh gets fixed, etc. And there is no rush, whenever you feel the urge to tinker is just great with me.

mateor avatar Jun 20 '13 14:06 mateor

How about this idea: since we would be calling batch.sh from within the auto-patcher, and the auto_patcher has set -a to pass variables to children, we could just test for an auto_patcher variable within batch.sh.

  if      [[ ! "$PATCHES_VERSION" == "" ]]; then
        # batch.sh was called from auto-patcher
  fi

I am implementing something similar from a kinda toy script I am writing to automate populating and processing the autopatche patches from builds. It would be best to use something we actually need in batch.sh, like ROMX.

mateor avatar Nov 19 '13 18:11 mateor