Skip to content
This repository has been archived by the owner on Jun 18, 2019. It is now read-only.

Fixing linux builds #22

Merged
merged 8 commits into from
Jun 9, 2017
Merged

Fixing linux builds #22

merged 8 commits into from
Jun 9, 2017

Conversation

GabeIsman
Copy link
Contributor

Fixes #20 🎊.

make clean-build produces a working .deb.

The global installs can be deleted because npm adds ./node_modules/.bin to the path for any npm run ... commands.

The build-app command can be deleted because I made it a part of dist a while back.

There were really three compounding issues here:

  1. The missing dynamic link.
  2. node-sass built for the wrong node bindings (I really don't know why, when they are installed from scratch). npm rebuild is just the magic dust you have to sprinkle everywhere to make npm behave itself.
  3. The sloppy .gitignore on my part was causing npm rebuild to fail.

I'll go ahead and the honorary problem that freaking webpack thinks missing modules are not a good enough reason to return a nonzero exit code.

This was initially running into an error because, I had been sloppy
with the .gitignore and added 'dist' instead of '/dist'. Apparently
.npmignore defaults to .gitignore if, so
`node_modules/electron/resources/dist` was invisible to `npm rebuild`.
Luckily I googled/guessed that quickly.
@conorsch
Copy link
Contributor

conorsch commented Jun 8, 2017

@GabeIsman, you are a hero! 🏅 Confirmed that .deb packages now run when building off this branch. A few questions:

  1. Don't we also need to update the rusty-secrets dependency in the top-level package.json file? It's been changed in the app/package.json file, but it seems unreasonable to me for the project to have conflicting dependencies.
  2. Should the top-level app/dist/ path be git-ignored? Hadn't noticed it before, but I'm definitely seeing it now.
  3. Can we start bumping the app version number? We're still declaring 0.0.1 for Sunder, but we've had a lot of changes, and I'd like to start tracking that progress to keep these build artifacts straight. 0.1.0 seems like a good target for public release.

Unfortunately there are still regressions at work: we lost the newer assets original presented in #17. Since having working builds again is vital to debugging the assets regression, I'm inclined to merge here and open separate tickets for fixing the assets.

As soon as you confirm can address the questions above, I'm fine with merge here.

@conorsch
Copy link
Contributor

conorsch commented Jun 9, 2017

Regarding asset regressions, looks like those are resolved in #24. This PR still needs to be merged in order to fix builds under Linux, but interested in @GabeIsman's thoughts on items 1-3 above.

@GabeIsman
Copy link
Contributor Author

@conorsch Great points on all 3!

They rusty-secrets version in the root package.json is only used for integration testing, but should definitely be kept in sync. Really we should merge the two package.json's so we don't have to worry about keeping that and the application version in sync between the two. electron-builder used to require the two-package.json project structure, but they've moved away from that somewhere in the last 13 versions that have come out this year (seriously). So we should be able to migrate to a single package.json.

Thank you for testing this so thoroughly. I was assuming all yesterday that #17 just wasn't merged but then it hit me sometime today that it had been. I'm glad you were able to sort out what went wrong!

@conorsch
Copy link
Contributor

conorsch commented Jun 9, 2017

Thanks @GabeIsman! Your updates look great. 🎊

@conorsch conorsch merged commit bb6ad0d into master Jun 9, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants