Skip to content
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

Merged
merged 1 commit into from
Mar 24, 2016
Merged

Build Updates #6111

merged 1 commit into from
Mar 24, 2016

Conversation

rossmcdonald
Copy link
Contributor

  • CHANGELOG.md updated
  • Rebased/mergable
  • Tests pass
  • Sign CLA (if not already signed)

Updates here:

  • Improved handling of tar and zip package outputs
  • Added support for statically-compiled binary outputs
  • Changed the nightly versioning convention from the Unix epoch to UTC YYYYMMDDhhmm

Fixes #5554 #5252

@rossmcdonald rossmcdonald added this to the 0.12.0 milestone Mar 24, 2016
@jsternberg
Copy link
Contributor

LGTM but you may want to have someone with more knowledge of the build script look at it too.

@rossmcdonald
Copy link
Contributor Author

/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):
Copy link
Contributor

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.

Copy link
Contributor Author

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.

Copy link
Contributor

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.

Copy link
Contributor Author

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. 😄

@rossmcdonald
Copy link
Contributor Author

@nathanielc I've changed the static flag for package_scripts to config_only for clarification.

@nathanielc
Copy link
Contributor

@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
@rossmcdonald
Copy link
Contributor Author

Rebased.

@nathanielc I also added one final change to the nightly date format. The format is now UTC %Y%m%d%H%M (or YYYYMMDDhhmm) over the unix epoch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants