Change the order of haxelib git submodule installation

[ninjamuffin99]

Putting this here as a bit of a draft / feedback.

Currently the haxelib git command does git clone --recursive which will install submodules alongside the clone. However you can run into a specific issue (openfl/lime#1798) related to potentially broken commits in the git history.
You can get around this specific issue by having transfer.fsckObjects = false in your git config, but based on my own testing it doesn't actually seem to apply this when doing the clone command, only once the repository is already initialized and when you're running git submodule update will it use the git config to ignore broken commits.

This PR moves the submodule installation to only run after the git clone finished successfully, and after checking out the specified branch/version.
It will run

• git submodule init
• git submodule sync --recursive
• git submodule update --recursive
  I may or may not have ran into issues when running it less verbosely (git submodule update --init --recursive initializes the submodules in the same command as update) but I believe ran into some issues there.

[kLabz]

Is init needed there?

For reference, that is what github's actions/checkout does:
https://github.com/actions/checkout/blob/9a9194f87191a7e9055e3e9b95b8cfb13023bb08/src/git-source-provider.ts#L235-L258

      // Checkout submodules
      core.startGroup('Fetching submodules')
      await git.submoduleSync(settings.nestedSubmodules)
      await git.submoduleUpdate(settings.fetchDepth, settings.nestedSubmodules)
      await git.submoduleForeach(
        'git config --local gc.auto 0',
        settings.nestedSubmodules
      )
      core.endGroup()

Which does support the idea of sync/update instead of --recursive

[ninjamuffin99]

seems like from that actions/checkout repo, its the submoduleUpdate() function that adds --init to the git submodule update command

async submoduleUpdate(fetchDepth: number, recursive: boolean): Promise<void> {
    const args = ['-c', 'protocol.version=2']
    args.push('submodule', 'update', '--init', '--force')
    if (fetchDepth > 0) {
      args.push(`--depth=${fetchDepth}`)
    }

    if (recursive) {
      args.push('--recursive')
    }

    await this.execGit(args)
  }

I think the order doesn't matter too much, the way I did it was a bit of an artifact of my R&D, but I'll update it to actually match the order of actions/checkout (sync, and then update + init)

[ninjamuffin99]

I just pushed a commit that also resolves #635 , where we read bytes from git's stderr output and just spit it out into our stdout as they come in, and then once it gets EOF it will continue on. Solves the hanging issue on my Mac, where it seems like it's waiting for something stderr/stdout related. And also this implementation does accomodate for the haxelib --quiet flag as well if you don't want a trillion lines of output.

[tobil4sk]

Thanks, this would also solve #593.

I just pushed a commit that also resolves #635 , where we read bytes from git's stderr output and just spit it out into our stdout as they come in, and then once it gets EOF it will continue on.

Would you be able to put the other changes in a separate PR?

[ninjamuffin99]

I suppose part of the specific hanging issue (on my mac that is) is it seems to want to spit the process stderr out somewhere when checking out submodules from lime, which has a broken commit in the cairo submodule from 15+ years ago. In our FunkinCrew/lime fork of lime we've added a .gitconfig file to ignore the broken fsck git object, however seems like when checking out the submodules it will hang and wait for something in the stderr to be consumed (even if the error doesn't actually exit the program, just a warning a bit). That's my theory a bit, and it's VERY specific to using the git version of lime on non-windows. On Windows it does that threading stuff to consume the stderr.

In anycase, I can and will split up the code into separate PRs!

[ninjamuffin99]

Would you be able to put the other changes in a separate PR?

Separate changes should now be in PR #641 !