-
-
Notifications
You must be signed in to change notification settings - Fork 156
Updated gulp build (added rpm build and fixed linux build on macOS) #105
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Seems to work fine on OSx (but no linux deb nor rpm packages produced by gulp, only the zip files). |
|
Removed that hard dmg dependency in And |
|
Yes, the method getLinuxPackageArch exists in bfc. Maybe has not been ported. |
mikeller
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've commented on the required changes. After this the .rpm package builds fine. I haven't tested installing it, since I am running ubuntu.
package.json
Outdated
| "license": "GPL-3.0", | ||
| "dependencies": {}, | ||
| "dependencies": { | ||
| "gulp-appdmg": "^1.0.3" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can't have this dependency in here - this will keep npm install from working on non-macosx. (Also, it is not a runtime dependency at all.)
The way that platform-dependent-modules works, and the consequence that package.json is modified every time a platform dependent module is installed, makes it hard and tedious to work with. I have tried some alternative, but wasn't able to find one that actually works better in this respect.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's odd, didn't move that dependency as I know it belongs in the platform dependent modules. I think the clean npm install did this automatically. I will check if I can reproduce it, if this is the case then this can happen more often when someone does a clean modules install.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, unfortunately platform-dependent-modules does this on npm install, making it a pain to develop with it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, saves me time to reproduce this. I will change it back and take extra care with future npm install clean installs
| var options = { | ||
| name: pkg.name, | ||
| version: pkg.version, | ||
| buildArch: getLinuxPackageArch('rpm', arch), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getLinuxPackageArch() needs to be defined (grab it from gulpfile.js of the betaflight-configurator.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, that one slipped through.
| rpmDest: RELEASE_DIR | ||
| }; | ||
|
|
||
| buildRpm(options, function(err, rpm) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
buildRpm needs to be defined: const buildRpm = require('rpm-builder').
Also, turns out that buildRpm does not like the -rc1 in the package version. We'll have to sanitise this for buildRpm, or stop using SemVer prereleases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks.
|
Made the changes as in the review comments, can someone try again if the rpm build works now. Thanks. |
|
Building on Linux and on OSx seems to produce packages OK. Not tried any actual install from any of the packages though, just built them. Also On OSx both |
|
On Linux. |
|
On OSx And |
| @@ -26,7 +26,7 @@ | |||
| "ansi-colors": { | |||
| "version": "1.0.1", | |||
| "resolved": "https://registry.npmjs.org/ansi-colors/-/ansi-colors-1.0.1.tgz", | |||
| "integrity": "sha512-yopkAU0ZD/WQ56Tms3xLn6jRuX3SyUMAVi0FdmDIbmmnHW3jHiI1sQFdUl3gfVddjnrsP3Y6ywFKvCRopvoVIA==", | |||
| "integrity": "sha1-6UxsMGAFr4tIIkAkHi896kuFX/M=", | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file comes up as changed after the npm install for me too, and needs updating to the latest version (posting the linux version of the file here since I suspect getting a vanilla package-lock.json on macosx is pretty much impossible due to the platform dependent module being installed):
|
|
|
Confirmed working without side effects on checked in files in linux. |
|
Strange, when I test it is still integrity warnings and some |
|
Integrity warnings on all platforms, and package-lock.json diffs, on Jenkins too: Also noticed that gulp errors out on linux32.rpm build, but does not return any error for Jenkins to detect. Same here and in BFC builds. Old problem (or is this expected?) I have not noticed. Error slipping by. I assumed/expected tools to return non-zero on errors. This npm-gulp-node-js-package-lockfile-world is strange... |
|
Interesting, the builds for Windows and linux work just fine here, and the artefacts seem to be working. Maybe your tests are broken? (I don't seem to be able to find the logs with build errors on your platform.) |
|
You have to drill down a bit on each platform to find the console outputs per build: |
|
@AndersHoglund: I see now. I think the warning up top is caused by some problem in the npm registry, and probably not something we can do anything about. Turns out the differences in |
|
OK, lets just merge #110 and see what Jenkins has to say about it. |
|
Seems Jenkins also need to purge |
|
@AndersHoglund: Yes, I think purging We should look into setting up travis (at least for one of the platforms), so we get integrated warnings. |
Fixed linux build on macOS (check if deb package exists)
Added RPM build for linux
Both fixes are as per BFC gulpfile.js
@mikeller: can you test if gulp successfully create the rpm package, i could't test that on my mac.