Re: [PATCH 1/8] Introduce CMake support for configuring Git on Linux

[Sibi Siddharthan <sibisiddharthan.github@gmail.com>]

On Sat, Apr 25, 2020 at 4:26 AM Danh Doan <congdanhqx@gmail.com> wrote:
>
> On 2020-04-25 02:36:12+0530, Sibi Siddharthan <sibisiddharthan.github@gmail.com> wrote:
> > Yes it does,
> > all you have to do is -DZLIB_ROOT=/path/to/zlib/root
>
> OK, I need to learn this syntax
>
> > > > +find_package(EXPAT)
> > > > +find_package(Iconv)
> > > > +find_package(Intl)
> > > > +
> > >
> > > > +if(NOT Intl_FOUND)
> > > > +     add_compile_definitions(NO_GETTEXT)
> > >
> > > find_package(Gettext)?
> > >
> >
> > find_package(Gettext) does not define libintl and libintl.h.
> > If NO_GETTEXT is not defined, it means that libintl.h is present.
> > So, we use find_package(Intl) for libintl
>
> People do weird things.
>
> But, I bet that a lot of people have system that have libintl.h
> installed, but they don't install gettext tools. ;-)
>
> Anyway, can I override NO_GETTEXT from command line?
>
> Let's say, I want to bootstrap a new distro?
> In the boostrap step, I don't want to build any i18n things,
> because the bootstrap package will be thrown away and rebuild again.
> I don't want to waste time on building such translation things.
>

So, you are trying to build two times and configuring it only once. Am I right?
If so I don't think you bootstrap with CMake. Will let you know if I
figure something out.

> >
> > > > +     if(NOT Iconv_FOUND)
> > > > +             add_compile_definitions(NO_ICONV)
> > > > +     endif()
> > > > +endif()
> > >
> > > ICONV_OMITS_BOM?
> > >
> >
> > Forgot about this, will add it.
>
> Thanks, my platform relies on this definition!
>
> > > > +
> > > > +include_directories(SYSTEM ${ZLIB_INCLUDE_DIRS})
> > > > +if(CURL_FOUND)
> > > > +     include_directories(SYSTEM ${CURL_INCLUDE_DIRS})
> > > > +endif()
> > > > +if(EXPAT_FOUND)
> > > > +     include_directories(SYSTEM ${EXPAT_INCLUDE_DIRS})
> > > > +endif()
> > > > +if(Iconv_FOUND)
> > > > +     include_directories(SYSTEM ${Iconv_INCLUDE_DIRS})
> > > > +endif()
> > > > +if(Intl_FOUND)
> > > > +     include_directories(SYSTEM ${Intl_INCLUDE_DIRS})
> > > > +endif()
> > > > +
> > > > +find_program(SH_EXE sh)
> > >
> > > We want to find usable sh, not just sh, no?
> > >
> > > It's matter on Solaris, HP-UX
> > >
> >
> > How do I check for this, can you help?
>
> Sorry, Please ask someone else, check Makefile to see who fixed build
> for Solaris and HP-UX.
>
> > Also, the script is not supported on Solaris and HP-UX.
>
> If Solaris or HP-UX is out of equation,
> I'll just write: SH_EXE=/bin/sh ;-)
>
>

This is exactly what the current definition does.

> >
> > > > +
> > > > +#default behaviour
> > > > +include_directories(${CMAKE_SOURCE_DIR})
> > > > +add_compile_definitions(GIT_HOST_CPU="${CMAKE_SYSTEM_PROCESSOR}")
> > > > +add_compile_definitions(SHA256_BLK INTERNAL_QSORT RUNTIME_PREFIX)
> > > > +add_compile_definitions(NO_OPENSSL SHA1_DC SHA1DC_NO_STANDARD_INCLUDES
> > > > +                     SHA1DC_INIT_SAFE_HASH_DEFAULT=0
> > > > +                     SHA1DC_CUSTOM_INCLUDE_SHA1_C="cache.h"
> > > > +                     SHA1DC_CUSTOM_INCLUDE_UBC_CHECK_C="git-compat-util.h" )
> > > > +list(APPEND compat_SOURCES sha1dc_git.c sha1dc/sha1.c sha1dc/ubc_check.c block-sha1/sha1.c sha256/block/sha256.c compat/qsort_s.c)
> > > > +
> > > > +
> > > > +add_compile_definitions(PAGER_ENV="LESS=FRX LV=-c"
> > > > +                     ETC_GITATTRIBUTES="etc/gitattributes"
> > > > +                     ETC_GITCONFIG="etc/gitconfig"
> > > > +                     GIT_EXEC_PATH="libexec/git-core"
> > > > +                     GIT_LOCALE_PATH="share/locale"
> > > > +                     GIT_MAN_PATH="share/man"
> > > > +                     GIT_INFO_PATH="share/info"
> > > > +                     GIT_HTML_PATH="share/doc/git-doc"
> > > > +                     DEFAULT_HELP_FORMAT="html"
> > > > +                     DEFAULT_GIT_TEMPLATE_DIR="share/git-core/templates"
> > > > +                     GIT_VERSION="${PROJECT_VERSION}.GIT"
> > > > +                     GIT_USER_AGENT="git/${PROJECT_VERSION}.GIT"
> > > > +                     BINDIR="bin"
> > > > +                     GIT_BUILT_FROM_COMMIT="")
> > >
> > > I wish I could verify this.
> > > Have you check this part on a default build system for a Linux distro,
> > > FreeBSD? For things started with "etc/"
> > >
> >
> > These are the values I got when I looked at the build logs from the
> > Makefile (make -n) in Linux and Windows.
> > Don't know about FreeBSD
>
> Linux will install configuration to /etc, Windows (by using msys),
> will also do that.
>
> *BSD port installs git with /usr/local prefix, and install
> configuration to /usr/local/etc
>

Aren't the above definitions relative paths?(With RUNTIME_PREFIX)
Once you decide to install, you have INSTALL_PREFIX(similar to
DESTDIR) with which you
can achieve the above.

> >
> > > > +
> > > > +set(FALLBACK_RUNTIME_PREFIX /home/$ENV{USER})
> > > > +add_compile_definitions(FALLBACK_RUNTIME_PREFIX="${FALLBACK_RUNTIME_PREFIX}")
> > > > +
> > > > +add_compile_definitions(PROCFS_EXECUTABLE_PATH="/proc/self/exe" HAVE_DEV_TTY )
> > >
> > > /proc/self/exe is Linux only, no?
> > >
> >
> > Yes, it is. This first patch only supports Linux build.
> >
> > > > +list(APPEND compat_SOURCES unix-socket.c)
> > > > +
> > > > +#header checks
> > > > +check_include_file(libgen.h HAVE_LIBGEN_H)
> > > > +if(NOT HAVE_LIBGEN_H)
> > > > +     add_compile_definitions(NO_LIBGEN_H)
> > > > +     list(APPEND compat_SOURCES compat/basename.c)
> > > > +endif()
> > > > +
> > > > +check_include_file(sys/sysinfo.h HAVE_SYSINFO)
> > > > +if(HAVE_SYSINFO)
> > > > +     add_compile_definitions(HAVE_SYSINFO)
> > > > +endif()
> > > > +
> > > > +check_c_source_compiles("
> > > > +#include <alloca.h>
> > > > +int
> > > > +main ()
> > > > +{
> > > > +char *p = (char *) alloca (2 * sizeof (int));
> > > > +     if (p) return 0;
> > > > +     return 0;
> > >
> > > All code path will return 0?
> > >
> >
> > This check is for a working alloca.h.
> > Some systems define alloca in malloc.h (Windows)
>
> Does CMake have a macro for check function in which include files?
>

The check functions is something that was used to match the automake behaviour.
If you want to check whether a function exists in a particulae header
file use check_symbol_exists().

> >
> > > > +}"
> > > > +HAVE_ALLOCA_H)
> > > > +if(HAVE_ALLOCA_H)
> > > > +     add_compile_definitions(HAVE_ALLOCA_H)
> > > > +endif()
> > > > +
> > > > +
> > > > +if(CURL_FOUND)
> > > > +     set(remote_exes
> > > > +             git-remote-https git-remote-ftp git-remote-ftps)
> > > > +     foreach(s ${remote_exes})
> > > > +             file(APPEND ${CMAKE_BINARY_DIR}/CreateLinks.cmake "file(CREATE_LINK git-remote-http ${s})\n")
>
> Now, I looked at this again.
> I didn't see the creatation of CreateLinks.cmake anywhere!
>
> > > > +             list(APPEND git_http_links ${CMAKE_BINARY_DIR}/${s})
> > > > +     endforeach()
> > > > +endif()
> > > > +
> > > > +add_custom_command(OUTPUT ${git_links} ${git_http_links}
> > > > +             COMMAND ${CMAKE_COMMAND} -P ${CMAKE_BINARY_DIR}/CreateLinks.cmake
> > > > +             DEPENDS git git-remote-http)
> > > > +add_custom_target(git-links ALL DEPENDS ${git_links} ${git_http_links})
> > > > \ No newline at end of file
> > >
> > > No new line at end of line always leave a bad taste in my mount!
> > >
>
> --
> Danh

Thank You,
Sibi Siddharthan