theypsilon / Update_All_MiSTer

All-in-one script for updating your MiSTer
GNU General Public License v3.0
607 stars 27 forks source link

Invalid arguments: #37

Closed Firebrandx closed 3 years ago

Firebrandx commented 3 years ago

Noticed 4 entries the log file for invalid arguments, and this is due to using characters that are not allowed. Here they are in sequential order:

mkdir: cannot create directory ‘/media/fat/games/AO486UART115200:4000000(Turbo 115200),MIDI’: Invalid argument

mkdir: cannot create directory ‘/media/fat/games/GAMEBOYSS3E000000:100000’: Invalid argument

mkdir: cannot create directory ‘/media/fat/games/GBASS3E000000:100000’: Invalid argument

error: cannot create /media/fat/cheats/TGFX16/Ranma ? (CD)(Jpn) [].zip Invalid argument

stefanerwinmayer commented 3 years ago

@Firebrandx I reported these on the regular updater repo if you would like to follow them.

Firebrandx commented 3 years ago

@Firebrandx I reported these on the regular updater repo if you would like to follow them.

My apologies, I was just told to report it here since it happened when I tried the update-all script.

stefanerwinmayer commented 3 years ago

@Firebrandx I didn't mean to imply you did the wrong thing. Might as well update-all script related - even though that's unlikely, I believe. I merely mentioned it so you can follow replies to my reported issues.

theypsilon commented 3 years ago

Indeed, it is from the regular updater but I'll try to find some time to investigate the issue too.

Didn't notice the errors on the games folders and cheats before. Thanks for reporting it.

stefanerwinmayer commented 3 years ago

I believe that's because they aren't tagged as errors which is another problem in itself. I just happened to actually look at the entire output which was obviously very boring.

Firebrandx commented 3 years ago

Since it was my first time building a MiSTer, I was intently gazing at the update commands as they scrolled by, and saw it. I freaked out because I didn't know if it was 'game-breaking' per se. ;-)

cdewit commented 3 years ago

Please see my comment containing a possible solution here: https://github.com/MiSTer-devel/Updater_script_MiSTer/issues/57#issuecomment-779513831

theypsilon commented 3 years ago

Thanks @cdewit I will check that solution a bit further when I have the time

Should I open a PR to that updater, or will you do it?

cdewit commented 3 years ago

@theypsilon Thank you for your reply. Please feel free to open a PR if you approve the proposed solution. I just wanted to try and help out and support the project.

You might also want to check out these comments I made on other issues I have looked into: https://github.com/MiSTer-devel/Updater_script_MiSTer/issues/59#issuecomment-781639064 https://github.com/MiSTer-devel/Updater_script_MiSTer/issues/59#issuecomment-781647836

I have tested my three fixes together on a clean install and they seem to resolve all the issues I observed but I'm not sure you approve all of the proposed solutions.

Thank you for all your efforts!

theypsilon commented 3 years ago

Thanks @cdewit . The fixes have been merged. Issues of this kind should be gone for now