Build system refactoring phase 3 #213

pspacek · 2016-11-07T17:35:28Z

This monster patch-set refactors most of build system and moves most of the logic from SPEC file to build system.

It is not yet complete, missing parts are:

Python 3 support (fix in Build: makerpms.sh generates Python 2 & 3 packages at the same time #272)
Client-only build is not supported (postponed, see ticket 6517)
IPA_VERSION_IS_GIT_SNAPSHOT does not work (fix in Build refactoring phase 5 #226)

These will be sorted out later on but the review of the patch set can begin.

MartinBasti · 2016-11-08T12:13:03Z

Why are there these whitespace errors?

git am ...
...
Applying: Build: stop build when a step in web UI build fails
Applying: Build: fix distribution of static files for web UI
.git/rebase-apply/patch:413: space before tab in indent.
    "./fs",
.git/rebase-apply/patch:414: space before tab in indent.
    "./fileUtils",
.git/rebase-apply/patch:415: space before tab in indent.
    "./process",
.git/rebase-apply/patch:418: space before tab in indent.
    "./stringify",
.git/rebase-apply/patch:419: space before tab in indent.
    "./version",
warning: squelched 49 whitespace errors
warning: 54 lines add whitespace errors.
Applying: Web UI: Remove offline version of Web UI

MartinBasti · 2016-11-08T12:16:22Z

I cannot build rpms

./makerpms.sh
...
config.status: executing libtool commands

                    IPA Server 4.4.90.201611071629GITbcd4f03
                    ========================

        prefix:                   /usr/local
        exec_prefix:              ${prefix}
        libdir:                   ${exec_prefix}/lib
        bindir:                   ${exec_prefix}/bin
        sbindir:                  ${exec_prefix}/sbin
        sysconfdir:               ${prefix}/etc
        localstatedir:            ${prefix}/var
        datadir:                  ${datarootdir}
        krb5rundir:               ${prefix}/var/run/krb5kdc
        systemdsystemunitdir:     /usr/lib/systemd/system
        source code location:     .
        compiler:                 gcc
        cflags:                   -g -O2
        LDAP libs:                -lldap_r -llber
        KRB5 libs:                -lkrb5 -lk5crypto -lcom_err
        KRAD libs:                -lkrad
        OpenSSL crypto libs:      -lcrypto
        Maintainer mode:          no

make: *** No rule to make target 'rpms'.  Stop.

MartinBasti · 2016-11-08T12:52:36Z

Build works for me, error was on my side

MartinBasti · 2016-11-08T13:51:58Z

Works for me

I read commits, it makes sense, works for me, but I'm not an autotools expert so I might miss something.

I'm curious about one commit: Build: cleanup unused LDIFs from install/share

do we have somewhere a documentation for Solaris that requires this file?

pspacek · 2016-11-08T16:34:56Z

This version supports lint target and all configuration options for linters listed in the design document.
Fixes in systemd-tmpfiles call from make install.
Updates .travis.yml to account for new method of RPM build.

MartinBasti · 2016-11-09T09:05:11Z

commit Build: add rpms target and makerpms.sh script misses makerpms.sh and ticket, and it looks like you forgot to squash this commit

tiran · 2016-11-09T09:23:25Z

The patch has some minor creases but works. Let's merge it to master and iron out the remaining small issues with PRs.

tiran · 2016-11-09T09:37:39Z

memo for me:

/freeipa*.tar.gz is not removed (fixed in Build: fix make clean to remove build artifacts from top-level directory #220)
MOSTLYCLEANFILES only cleans ipasetup.py[co] but keeps pycache and other pyc/pyo. add clean-local: rm -rf *.pyc *.pyc __pycache__ (fixed in Build: fix make clean to remove build artifacts from top-level directory #220)
Makefile.python.am clean-local has -delete and -exec. AFAIK only one action is supported.
neither clean nor distclean removes /dist and /rpmbuild (fixed in Build: fix make clean to remove build artifacts from top-level directory #220)
autoconf and automake files are not removed (Makefile.in, /config.sub ...)
add ipasetup.py to dist_noinst_SCRIPTS ?

Version information is now in VERSION.m4 instead of VERSION. Makefile target version-update was minimized and configure can be run before make. Makefile temporarily contains hardcoded version which has to match the one specified in VERSION.m4. This is preparatory step which will allow us to replace hand-made Makefile with one generated by Automake. https://fedorahosted.org/freeipa/ticket/6418

The neither build nor dist targets work completely. This is temporary breakage enabling further work. https://fedorahosted.org/freeipa/ticket/6418

The name in setup.py should match real name of the module. It will be used by the build system later on. https://fedorahosted.org/freeipa/ticket/6418

This version builds only one version of Python packages. If you want to build for Python 2 & 3 call configure twice using different --with-python or specify PYTHON variable when calling make. dist-hook is using SOURCES.txt file from egg-info. According to Petr Viktorin this should be enough for our purposes and avoids need to create plugins for setuptools. Currently VPATH builds do not work for various reasons. This should be fixed later on. Most credit goes to these guys: Christian Heimes <cheimes@redhat.com> Petr Viktorin <pviktori@redhat.com> Kevin Brown <kevin@kevin-brown.com> https://fedorahosted.org/freeipa/ticket/6418

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

Most of the logic was unnecessary and wrong. This caused make install to fail. This commit removes unnecessary declarations and creates static library which is not installed. make install in asn1 subdirectory is now passing (and doing nothing). https://fedorahosted.org/freeipa/ticket/6418

This makes easier to use --prefix with make install. https://fedorahosted.org/freeipa/ticket/6418

This is step forward working VPATH builds which cleanly separate sources and build artifacts. It makes the system cleaner and easier to understand. Python and web UI likely require more work to make VPATH builds working. https://fedorahosted.org/freeipa/ticket/6418

Some Makefile.am files were apparently created by copy-pasting other files. As a result, some Makefiles require non-existing README files. Remove this to fix dist target. https://fedorahosted.org/freeipa/ticket/6418

The name from configure.ac is used when generating tarball. https://fedorahosted.org/freeipa/ticket/6418

It now distributes po, pot, Makefile.in, and associated text files. https://fedorahosted.org/freeipa/ticket/6418

By default automake does not distribute man pages. This marks then with dist_ prefix to force their distribution in tarball. https://fedorahosted.org/freeipa/ticket/6418

… directory Static files from Git which are not touched by the build system have to be explicitly listed in Makefile.am so they get into tarball. ipa script was missing on installed systems for the same reason. https://fedorahosted.org/freeipa/ticket/6418

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

Static files from Git which are not touched by the build system have to be explicitly listed in Makefile.am so they get into tarball. EXTRA_DIST lists whole sub-directories with static files. This is not ideal but we do not have enough time to fix it properly. Dojo builder patch files were renames to shorten their name. The original names were exceeding autotools length limit. https://fedorahosted.org/freeipa/ticket/6418

The offline version does not work for some time already. I'm removing it right now to get rid of garbage which clutters build system. https://fedorahosted.org/freeipa/ticket/6447

BASH completion is now installed by make install. https://fedorahosted.org/freeipa/ticket/6418

FreeIPA has hard dependency on systemd for a long time already. SystemV directory was just polluting the tarball (while being useless). https://fedorahosted.org/freeipa/ticket/6418

At the same time, I've renamed tmpfilesd config file to static name "ipa.conf" instead of using package-specific name. It had no purpose and just complicated build and packaging. Variable substitution into configuration has to be done in Makefile and not in Autoconf as documented in: Autoconf v2.69 manual chapter 4.8.2 Installation Directory Variables: ... Most of these variables have values that rely on prefix or exec_prefix. ... Similarly, you should not rely on AC_CONFIG_FILES to replace bindir and friends in your shell scripts and other files; instead, let make manage their replacement. https://fedorahosted.org/freeipa/ticket/6418

Directory creating was moved from SPEC file to install-data-hook. At the same time, it is using systemd-tmpfiles to create the directories so we do not risk any inconsistency between SPEC file and tmpfilesd configuration. systemd-tmpfiles call is non-critical on purpose: The build would fail when run under unprivileged user because systemd-tmpfiles tries to change ownership. Luckily it creates all the files and just do not change ownership so it works even under unprivileged user. Interestingly, systemd-tmpfiles continues if user does not have sufficient permissions to change ownership but fails if target username does not exist at all. For this reason there is BuildRequires on httpd. https://fedorahosted.org/freeipa/ticket/6418

Makefile in doc subdirectory should be integrated into the main build system but I do not have time to do it now. For now it is enough to distribute everything. https://fedorahosted.org/freeipa/ticket/6418

At the time of this writting https://fedoraproject.org/wiki/Packaging:Guidelines#Manpages says this: When installing man pages, note that they should be installed uncompressed as the build system will compress them as needed. The compression method may change, so it is important to reference the pages in the %files section with a pattern that takes this into account: %{_mandir}/man1/foo.1* Removing the compression also allows to remove several install-data-hook targets from Makefile.am files. https://fedorahosted.org/freeipa/ticket/6418

… SPEC These directories were not used at all. https://fedorahosted.org/freeipa/ticket/6418

The only useful file is /etc/ipa/kdcproxy/kdcproxy.conf so I've removed the other copy of the file in /usr. https://fedorahosted.org/freeipa/ticket/6418

make rpms and ./makerpms.sh will produce the same RPM packages. The advantage of makerpms.sh is that it will take care of initial autoreconf & configure phases as needed. rpm-build-4.13.0-1.fc24.x86_64 broke parallel build of RPMs. If you get error INTERNAL: Exiting with 1 jobserver tokens available; should be 8! undefine the MAKEFLAGS variable and do not specify neither -j nor -l. https://fedorahosted.org/freeipa/ticket/6418

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

pspacek · 2016-11-09T11:15:39Z

I've fixed cases where make generated some build artifacts but make clean did not remove them. These were distinct from Christian comments, I will fix these later on.

pspacek · 2016-11-09T11:18:27Z

@tiran

add ipasetup.py to dist_noinst_SCRIPTS ?
ipasetup.py file is auto-generated from ipasetup.py.in so it should not be part of distibution tarball. I've marked this item as "done".

pspacek · 2016-11-09T11:26:42Z

@tiran
I've tested the find command and it works. The trick is -o which acts like OR and allows you to specify different conditions and associate different actions to them. I'm going to check the checkbox as well :-)

MartinBasti · 2016-11-09T12:02:53Z

ACK

martbab · 2016-11-09T12:09:02Z

make lint and make dist were generating files which were not removed by make clean. https://fedorahosted.org/freeipa/ticket/6418

pspacek · 2016-11-09T13:03:23Z

@tiran

autoconf and automake files are not removed (Makefile.in, /config.sub ...)

According to Automake manual section 13 What Gets Cleaned we must not remove files necessary for ./configure.

As far as I can tell from testing, make distclean + PR #220 leaves behind only files generated by autoreconf so we should not remove any of them. It would prevent users from running configure again.

pspacek force-pushed the build-steps branch 2 times, most recently from ecc26a5 to acb1929 Compare November 8, 2016 16:33

pspacek force-pushed the build-steps branch from acb1929 to c516f43 Compare November 8, 2016 16:45

tiran approved these changes Nov 9, 2016

View reviewed changes

tiran added the ack Pull Request approved, can be merged label Nov 9, 2016

pspacek force-pushed the build-steps branch from c516f43 to 6c7a527 Compare November 9, 2016 09:49

pspacek added 16 commits November 9, 2016 12:13

Build: replace hand-made Makefile with one generated by Automake

91cf671

The neither build nor dist targets work completely. This is temporary breakage enabling further work. https://fedorahosted.org/freeipa/ticket/6418

Build: fix module name in ipaserver/setup.py

538290f

The name in setup.py should match real name of the module. It will be used by the build system later on. https://fedorahosted.org/freeipa/ticket/6418

Build: fix ipaplatform detection for out-of-tree builds

cbefae2

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

Build: respect --prefix for systemdsystemunitdir

1577fc8

This makes easier to use --prefix with make install. https://fedorahosted.org/freeipa/ticket/6418

Build: remove non-existing README files from Makefile.am

a6fd70f

Some Makefile.am files were apparently created by copy-pasting other files. As a result, some Makefiles require non-existing README files. Remove this to fix dist target. https://fedorahosted.org/freeipa/ticket/6418

Build: rename project from ipa-server to freeipa

91e4ae3

The name from configure.ac is used when generating tarball. https://fedorahosted.org/freeipa/ticket/6418

Build: fix distdir target for translations

b1a2ab9

It now distributes po, pot, Makefile.in, and associated text files. https://fedorahosted.org/freeipa/ticket/6418

Build: fix man page distribution

df0c1cb

By default automake does not distribute man pages. This marks then with dist_ prefix to force their distribution in tarball. https://fedorahosted.org/freeipa/ticket/6418

Build: stop build when a step in web UI build fails

5772a72

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

Web UI: Remove offline version of Web UI

9ed8f83

The offline version does not work for some time already. I'm removing it right now to get rid of garbage which clutters build system. https://fedorahosted.org/freeipa/ticket/6447

pspacek added 16 commits November 9, 2016 12:13

Build: integrate contrib directory into build system

a0c46df

BASH completion is now installed by make install. https://fedorahosted.org/freeipa/ticket/6418

Build: remove init/SystemV directory

7f73a55

FreeIPA has hard dependency on systemd for a long time already. SystemV directory was just polluting the tarball (while being useless). https://fedorahosted.org/freeipa/ticket/6418

Build: distribute doc directory

10553f9

Makefile in doc subdirectory should be integrated into the main build system but I do not have time to do it now. For now it is enough to distribute everything. https://fedorahosted.org/freeipa/ticket/6418

Build: remove unused dirs /var/cache/ipa/{sysupgrade,sysrestore} from…

efb7baf

… SPEC These directories were not used at all. https://fedorahosted.org/freeipa/ticket/6418

Build: fix KDC proxy installation and remove unused kdcproxy.conf

177b4fe

The only useful file is /etc/ipa/kdcproxy/kdcproxy.conf so I've removed the other copy of the file in /usr. https://fedorahosted.org/freeipa/ticket/6418

Build: remove obsolete instructions about BuildRequires from BUILD.txt

1d94f5d

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

Build: add Python lint target

858e7f0

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

Build: add JS lint target

8cfe9c4

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

Build: add makeaci lint target

72a5f11

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

Build: add makeapi lint target

814de1e

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

Build: add polint target for i18n tests

40aab00

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

Build: fix make clean for web UI

b3deb0e

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

pspacek force-pushed the build-steps branch from 6c7a527 to b3deb0e Compare November 9, 2016 11:13

martbab added the ack Pull Request approved, can be merged label Nov 9, 2016

pspacek assigned MartinBasti Nov 9, 2016

martbab added the pushed Pull Request has already been pushed label Nov 9, 2016

martbab closed this Nov 9, 2016

pspacek deleted the build-steps branch November 9, 2016 12:22

pspacek referenced this pull request in pspacek/freeipa Nov 9, 2016

Build: fix make clean to remove build artifacts from top-level directory

e2688d9

make lint and make dist were generating files which were not removed by make clean. https://fedorahosted.org/freeipa/ticket/6418

pspacek mentioned this pull request Nov 9, 2016

Build: fix make clean to remove build artifacts from top-level directory #220

Closed

Build system refactoring phase 3 #213

Build system refactoring phase 3 #213

pspacek commented Nov 7, 2016 •

edited

MartinBasti commented Nov 8, 2016

MartinBasti commented Nov 8, 2016

MartinBasti commented Nov 8, 2016

MartinBasti commented Nov 8, 2016

pspacek commented Nov 8, 2016

MartinBasti commented Nov 9, 2016

tiran commented Nov 9, 2016

tiran commented Nov 9, 2016 •

edited by pspacek

pspacek commented Nov 9, 2016

pspacek commented Nov 9, 2016

pspacek commented Nov 9, 2016

MartinBasti commented Nov 9, 2016

martbab commented Nov 9, 2016

pspacek commented Nov 9, 2016 •

edited

Build system refactoring phase 3 #213

Build system refactoring phase 3 #213

Conversation

pspacek commented Nov 7, 2016 • edited

MartinBasti commented Nov 8, 2016

MartinBasti commented Nov 8, 2016

MartinBasti commented Nov 8, 2016

MartinBasti commented Nov 8, 2016

pspacek commented Nov 8, 2016

MartinBasti commented Nov 9, 2016

tiran commented Nov 9, 2016

tiran commented Nov 9, 2016 • edited by pspacek

pspacek commented Nov 9, 2016

pspacek commented Nov 9, 2016

pspacek commented Nov 9, 2016

MartinBasti commented Nov 9, 2016

martbab commented Nov 9, 2016

pspacek commented Nov 9, 2016 • edited

pspacek commented Nov 7, 2016 •

edited

tiran commented Nov 9, 2016 •

edited by pspacek

pspacek commented Nov 9, 2016 •

edited