Skip to content

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

Merged
alerque merged 2 commits into
alerque:masterfrom
shiftkey:patch-1
Aug 31, 2021
Merged

[github-desktop] update PKGBUILD to workaround missing submodules#22
alerque merged 2 commits into
alerque:masterfrom
shiftkey:patch-1

Conversation

@shiftkey

@shiftkey shiftkey commented Aug 29, 2021

Copy link
Copy Markdown
Contributor

Spotted this comment over in the AUR feed:

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

👋 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 immackay/github-desktop-aur#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
shiftkey marked this pull request as ready for review August 29, 2021 16:20
@shiftkey

Copy link
Copy Markdown
Contributor Author

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

Copy link
Copy Markdown
Contributor Author

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. shiftkey/desktop#587 is the fix, which I need to confirm with the Flatpak maintainers to confirm I won't break their case...

@alerque

alerque commented Aug 31, 2021

Copy link
Copy Markdown
Owner

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.

@alerque
alerque merged commit e4da8e0 into alerque:master Aug 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants