-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Build Updates #6111
Build Updates #6111
Conversation
LGTM but you may want to have someone with more knowledge of the build script look at it too. |
/cc @nathanielc This PR this fixes issues with zips and tarballs, which will be ported to Kapacitor assuming everything looks okay. |
@@ -109,16 +109,20 @@ def create_package_fs(build_root): | |||
create_dir(os.path.join(build_root, d)) | |||
os.chmod(os.path.join(build_root, d), 0755) | |||
|
|||
def package_scripts(build_root): | |||
def package_scripts(build_root, static=False): |
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.
So if I understand correctly by passing static here we don't copy all the service scripts etc just the config. Then for builds marked static and all windows build we set static=True since these extra scripts don't apply.
The name static
for this method is confusing maybe we should name this parameter config_only
?
Then this code below dosen't read as all windows builds are static:
if platform == 'windows' or static:
package_scripts(build_root, static=True)
It is a simple fix but it was quite confusing at first.
Also why do static build not need the extra service scripts etc? Is it because they are intended for use in apline linux? If so a comment explaining that would be helpful.
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.
Also why do static build not need the extra service scripts etc? Is it because they are intended for use in apline linux? If so a comment explaining that would be helpful.
That is correct. The only people who should be using the static builds are people who just need binaries, so Alpine users and Windows users are the first ones who come to mind. Do you think all tars and zips should be this way?
The name static for this method is confusing maybe we should name this parameter config_only?
Agreed. I'll change it. I was originally thinking of 'static' packages (just plain zip, tar), but I can see how it's misleading and/or vague.
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.
Do you think all tars and zips should be this way?
No, I think it is useful to provide the scripts in tars, zips since they have value, at least in the linux versions. To me the contents of a package (tar, deb, zip etc) depends on the platform and not the package type. No matter the package type provide the relevant content for the platform.
Why do Windows users want static builds? I get why they don't care about service scripts etc but not static builds.
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.
Why do Windows users want static builds?
Sorry, Windows builds won't be statically compiled, but they will use the static
flag (soon to be config_only
) for the packaging filesystem portion. Another reason why static
should not have been used as the flag name. 😄
@nathanielc I've changed the |
@rossmcdonald Looks good. |
- Improved handling of tar and zip package outputs - Added support for statically-compiled binary outputs - Modifyed the nightly version format to be more human-readable
aa97dfc
to
eb172ba
Compare
Rebased. @nathanielc I also added one final change to the nightly date format. The format is now UTC |
Updates here:
YYYYMMDDhhmm
Fixes #5554 #5252