-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Add spec file to build rpm #244
Conversation
Unit testing passed. |
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.
Well done!
Overall the pr looks good to me. I just have a few minor suggestions.
package/build_deps.sh
Outdated
@@ -0,0 +1,10 @@ | |||
#!/bin/sh | |||
|
|||
# download deps for build nebula |
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.
Should we check the package version as well?
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 had only tested using Federa29 for now, I will test in other system of variant version and check the package version requirements as soon as the experimental envioronment is ready.
package/make_srpm.sh
Outdated
bindir="$prefix/bin" | ||
datadir="$prefix/scripts" | ||
sysconfdir="$prefix/etc" | ||
version="1.0.0" |
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.
The version is used in both SRPM and RPM spec. I wonder if we could pull the version out to a central location
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.
make_srpm.sh will pass the version parameter to nebula.spec. Besides, i think that we could parse version from tag name or user input.
P.S. We need to confirm a version format.
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.
Sure, sounds good to me. Maybe a TODO comment for now and implement it later
|
||
# download deps | ||
|
||
sudo yum -y install autoconf automake libtool cmake bison unzip boost gperf |
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.
Same as build_deps.sh, we might want to check the dependents' versions
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.
ditto
Unit testing failed. |
Unit testing passed. |
package/make_srpm.sh
Outdated
@@ -0,0 +1,65 @@ | |||
#!/bin/bash | |||
|
|||
# used to build nebula rpm flles |
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.
typo
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,thank you!
rpmbuilddir=$OPTARG | ||
;; | ||
?) echo "Invalid option, use default arguments" | ||
;; |
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.
alignment
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,thank you!
Unit testing passed. |
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.
LGTM
Awesome job and thanks for the quick response 👍
Unit testing passed. |
|
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.
Good job!
Unit testing passed. |
1 similar comment
Unit testing passed. |
* add spec file to build rpm * Address sherman-the-tank's comments * Fix compilation failure * Address steppenwolfyuetong's comments
* add spec file to build rpm * Address sherman-the-tank's comments * Fix compilation failure * Address steppenwolfyuetong's comments
* Remove Common Clause License (vesoft-inc#3263) * Remove Common Clause 1.0 License * Again * Cleanup CC * Remove common clause 1.0 Co-authored-by: Yee <2520865+yixinglu@users.noreply.github.com>
package/build_deps.sh : install deps for build nebula
package/rpm_deps.sh : install deps for rpm
package/make_srpm.sh : the scripts to make rpm packet
package/nebula.spec : the config file for rpmbuild
src/daemons/CMakeLists.txt : add storaged and metad when install