alerque / aur

Package sources for all the AUR packages I either maintain, co-maintain, or fork.
https://wiki.archlinux.org/index.php/Unofficial_user_repositories#alerque
46 stars 26 forks source link

[github-desktop] update PKGBUILD to workaround missing submodules #22

Closed shiftkey closed 3 years ago

shiftkey commented 3 years ago

Spotted this comment over in the AUR feed:

Something goes wrong during the webpack stage in relation to emoji or something.

:wave: GitHub Desktop Linux maintainer here. I was working previously with immackay on this package, and I think I know what's happening here. The tooling has a post-install script that ensures it has submodules on disk, but we wrapped that in an online check to support Flatpak, which was recently enabled:

https://github.com/shiftkey/desktop/blob/c04d1aeb46aa87953531aa83ce96d1c120281b21/script/post-install.ts#L56-L66

  if (isOffline()) {
    result = spawnSync(
      'git',
      ['submodule', 'update', '--recursive', '--init'],
      options
    )
    ...
  }

Where isOffline() is looking for an OFFLINE=1 environment variable:

https://github.com/shiftkey/desktop/blob/c04d1aeb46aa87953531aa83ce96d1c120281b21/script/post-install.ts#L15-L18

/** Check if the caller has set the OFFLINe environment variable */
function isOffline() {
  return process.env.OFFLINE === '1'
}

But that's been in place with the 2.9.0-linux4 build you have working, so perhaps something's changed upstream of me that I didn't expect.

Anyway, the workaround for this is to ensure the submodules are created before doing yarn build:prod. I thought DLAGENTS was the right place to add --recurse-submodules but I don't see that working when I tested it on https://github.com/immackay/github-desktop-aur/pull/16, and had to fall back to this approach.

I've bumped the package release to 2.9.2-linux1 to confirm this is resolved, but I've verified it over in the immackay/github-desktop-aur repo (which runs the changes through makepkg), but I'm also surprised that this isn't requiring Node 14 or later for building. But I guess we'll see how CI goes...

shiftkey commented 3 years ago

Tested the PKGBUILD in a VM and it's working as expected:

Will dig into better approaches for the submodule issue, but for now at least we're unblocked on this latest release...

shiftkey commented 3 years ago

Found the root cause - that check in the post-install is inverted incorrectly - only running when OFFLINE=1 is set, which is the opposite of what we intended. https://github.com/shiftkey/desktop/pull/587 is the fix, which I need to confirm with the Flatpak maintainers to confirm I won't break their case...

alerque commented 3 years ago

Hey @shiftkey thanks a lot for this! It saved me a lot of time trying to track down what was happening. I've taken your fix and reworked it a little bit to be closer to Arch packaging guidelines, but the gist of the fix is the same.

Ideally this would get worked out even further so all the node modules were fetched in prepare() and the build & package stages could run in offline mode, but at least fixing these module downloads is a step forward not a regression.