libest icon indicating copy to clipboard operation
libest copied to clipboard

Improve build script

Open deviance opened this issue 4 years ago • 5 comments

  • java/jni/client.c: add support for OpenSSL 1.1
  • Add --{enable,disable}-examples flag to toggle examples compilation
  • Add --with-system-libsafec flag to link against system libsafec
  • configure.ac: Fix AC_ARG_ENABLE/AC_ARG_WITH macros

deviance avatar Jul 12 '20 21:07 deviance

@rpb5bnc Hey Pete Beal, I'd like to have this reviewed. Thanks

deviance avatar Jul 13 '20 20:07 deviance

Pete is on vacation and should be returning sometime next week….

On Jul 13, 2020, at 4:34 PM, Aleksandr Makarov <[email protected]mailto:[email protected]> wrote:

@rpb5bnchttps://github.com/rpb5bnc Hey Pete Beal, I'd like to have this reviewed. Thanks

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/cisco/libest/pull/81#issuecomment-657779618, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ABZ5VGXH3JA3SG3UXNPDAULR3NVVFANCNFSM4OX7TVCA.

dieseldad60 avatar Jul 13 '20 20:07 dieseldad60

Hey guys, @rpb5bnc @dieseldad60 Care to look at this?

deviance avatar Jul 27 '20 17:07 deviance

It has been being looked at for the past week.  

The libEST repo is based upon an internal version of libEST, and the
internal version has different requirements then the external
implementation.  In order to efficiently maintain the two versions
we work hard to keep them the same whenever possible.  For example,
there has generally not been a widely used implementation of the
SafeC APIs in the industry, so the inclusion of the stub SafeC
folder and its use in the external version of liBEST is used to
allow the EST specific code to be identical in this regard in both
internal and external versions.  In the case of these new
PRs/issues, the suggested changes understandably cannot take into
account and satisfy the internal requirements.  

It was not stated which SafeC package had been installed in the
system that you were using, however, I really only know of one out
on the Internet and it's this one,

https://centos.pkgs.org/7/epel-x86_64/libsafec-3.3-5.el7.x86_64.rpm.html

If this is the case, then I'm very familiar with this version of
SafeC, or at least, its earlier incarnations.  Bo Berry was the
original author of this code and I was one of the two developers
required to use it within Cisco in the early 2000s before it became
widely adopted across the company.  I even vaguely remember the fact
that it does not properly include system header files.  Our internal
version of SafeC is generally not installed as an RPM package so the
use of, 

PKG_CHECK_MODULES([libsafec],
    [libsafec])
    
    will not work correctly for our internal uses.  Using the
        PKG_CHECK_MODULE() method to determine if a package is
        installed will not work when the SafeC library has been
        built specifically for a solution versus just installing the
        pre-built package.  Almost all of our product
    teams build our SafeC library for their product and/or platform
    and therefore the package manager will not know about the
    installation.  We use the standard method in configure to test
    for a package by calling an API into the package.  However
    again, the SafeC API we use for this retrieves the version of
    the SafeC library, and this API is not part of the APIs in
    Appendix K, so this won't work for outside or a strictly
    standard implementation of SafeC.  So, taking in these changes
    will cause problems inside and just accepting and merging these
    changes to the external implementation causes the two
    implementations to again diverge further, causing higher
    maintenance in the future.
    
    In regards to the Java based interface to libEST, we had dropped
    support of that internally several years ago.  Our internal
    automatic test pipelines no longer test the CiscoJEST/JEST
    code.  It was included in this recent update in Github as a way
    to make it available on an as is basis.  I do see that it needs
    a bit of change to make it work with the latest build
    environment, including OpenSSL 1.1.1.  I've looked at these
    issues and will make the necessary changes.  Again though, we
    generally did the 1.0.2 to 1.1.1 changes in a slightly different
    way than has been proposed here.
    
    We'll continue to look into the issues and should be able to
    make the necessary changes shortly.
    
    Pete
  
On 7/27/20 1:33 PM, Aleksandr Makarov
  wrote:


  
  Hey guys, @rpb5bnc
    @dieseldad60
    Care to look at this?
  —
    You are receiving this because you were mentioned.
    Reply to this email directly, view it on GitHub, or unsubscribe.
  [

{ "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/cisco/libest/pull/81#issuecomment-664534879", "url": "https://github.com/cisco/libest/pull/81#issuecomment-664534879", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]

rpb5bnc avatar Jul 27 '20 19:07 rpb5bnc

The libEST repo is based upon an internal version of libEST, and the internal version has different requirements then the external implementation. In order to efficiently maintain the two versions we work hard to keep them the same whenever possible. For example, there has generally not been a widely used implementation of the SafeC APIs in the industry, so the inclusion of the stub SafeC folder and its use in the external version of liBEST is used to allow the EST specific code to be identical in this regard in both internal and external versions. In the case of these new PRs/issues, the suggested changes understandably cannot take into account and satisfy the internal requirements.

Sorry, but I do not see how introduction of an optional configuration flag, which, if not given, falls back to a well-known existing option, may be a problem. Appreciate if you clarify which requirements are needed to be satisfied in this case.

Our internal version of SafeC is generally not installed as an RPM package so the use of,

PKG_CHECK_MODULES([libsafec],
    [libsafec])

    will not work correctly for our internal uses.  Using the
        PKG_CHECK_MODULE() method to determine if a package is
        installed will not work when the SafeC library has been
        built specifically for a solution versus just installing the
        pre-built package.  Almost all of our product
    teams build our SafeC library for their product and/or platform
    and therefore the package manager will not know about the
    installation.  We use the standard method in configure to test
    for a package by calling an API into the package. 

The PKG_CHECK_MODULE call is not intended to be a solution suitable for both built-in/system libsafec CFLAGS acquisition. The PKG_CHECK_MODULE is used as a method of CFLAGS acquisition only when --with-system-libsafec is given to ./configure, and if not, then the hard-coded CFLAGS for the stubbed SafeC are used.

However again, the SafeC API we use for this retrieves the version of the SafeC library, and this API is not part of the APIs in Appendix K, so this won't work for outside or a strictly standard implementation of SafeC. So, taking in these changes will cause problems inside and just accepting and merging these changes to the external implementation causes the two implementations to again diverge further, causing higher maintenance in the future.

For system libsafec, I've used a https://github.com/rurban/safeclib which you seem to reference as CentOS RPM. I've not seen API incompatibilities when switching to it for linking.

deviance avatar Jul 28 '20 00:07 deviance