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 refactoring phase 5 #226

Closed
wants to merge 7 commits into from
Closed

Conversation

pspacek
Copy link
Contributor

@pspacek pspacek commented Nov 10, 2016

This PR fixes IPA_VERSION_IS_GIT_SNAPSHOT option and vendor version passing from SPEC to configure. At also contains minor cleanup and srpm target which is used by Coverity.

@pspacek
Copy link
Contributor Author

pspacek commented Nov 10, 2016

I've added missing files to .gitignore.

@pspacek
Copy link
Contributor Author

pspacek commented Nov 10, 2016

Rebased on top of current master.

@tkrizek
Copy link
Contributor

tkrizek commented Nov 10, 2016

I don't understand Makefiles, but I tested building the git snapshots and srpms and it works. Just a few notes:

  • make clean removes only the most recently created tarball
  • there is not much of a time difference when building with IPA_VERSION_IS_GIT_SNAPSHOT - it takes about 1m50s instead of 1m35s when it's turned off. Is this expected?

Default format used by Autotools limits length of paths to
99 characters. This is not enough for tarballs with Git snapshots.

https://fedorahosted.org/freeipa/ticket/6418
… build

This is a huge hack. rpms target will touch VERSION.m4 file. This change
is then detected by automake Makefiles which subsequently re-execute configure
and make.

We have to workaround fact that variables in new make targets
(executed after new configure) are different than original ones.

Also, we have to 'bake-in' precise snapshot version from Git to
VERSION.m4 inside of RPM tarball so the RPM does not depend on git
anymore.

All this magic slows build down a bit.
Do not enable IPA_VERSION_IS_GIT_SNAPSHOT if you want fastest possible builds.

The option IPA_VERSION_IS_GIT_SNAPSHOT is now enabled by default as it
was before we started the build system refactoring effort.

https://fedorahosted.org/freeipa/ticket/6418
Python setuptools started to warn about forward incompatibility.
Now we are following PEP440 so it should not cause any problems
with future versions of setuptools.

https://fedorahosted.org/freeipa/ticket/6418
This is required in order to bake-in precise vendor version to
version.py.

https://fedorahosted.org/freeipa/ticket/6418
@pspacek
Copy link
Contributor Author

pspacek commented Nov 11, 2016

  • I've fixed the tarball removal so it deletes all of them.
  • The slow-down ~ 15 % is a nice surprise, I was under impression that it will be even slower.
  • I forgot to enable it by default as it was before the refactoring, this is fixed as well.

Please re-review.

@tkrizek tkrizek added the ack Pull Request approved, can be merged label Nov 11, 2016
@MartinBasti MartinBasti added the pushed Pull Request has already been pushed label Nov 11, 2016
@pspacek pspacek deleted the build-phase-5 branch November 11, 2016 11:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ack Pull Request approved, can be merged pushed Pull Request has already been pushed
Projects
None yet
3 participants