Add Ubuntu x86_64 FLEX_LIB_DIR into Makefile.defs

[jimmielin]

This is a minor fix that matches Ubuntu and x86_64 against uname -a to set FLEX_LIB_DIR=/usr/lib/x86_64-linux-gnu for these systems. This should support a pretty wide range of Ubuntu-based linux distros out-of-the-box. (libfl-dev still needs to be installed)

I also updated the "error message" to better clarify that Makefile.defs needs to be changed, if libfl.a is not found.

Perhaps we should also support USER_FLEX_LIB_DIR so FLEX_LIB_DIR in this file can be overridden without changing Makefile.defs in the future? Just to avoid accidentally committing user changes to Makefile.defs in the future, I almost got caught by surprise...

[laestrada]

@jimmielin I believe the CI checks fail because there is a sed command in the Dockerfile that was added to set FLEX_LIB_DIR for the ci tests.

With this update the sed command now causes FLEX_LIB_DIR = /lib/x86_64-linux-gnu/x86_64-linux-gnu.

The CI test can be fixed by updating the Dockerfile line from RUN sed -i 's/FLEX_LIB_DIR = \/usr\/lib/FLEX_LIB_DIR = \/lib\/x86_64-linux-gnu/' /kpp/src/Makefile.defs to RUN sed -i '0,/FLEX_LIB_DIR = \/usr\/lib/{s/FLEX_LIB_DIR = \/usr\/lib/FLEX_LIB_DIR = \/lib\/x86_64-linux-gnu/}' /kpp/src/Makefile.defs, which will only update the first occurrence of FLEX_LIB_DIR =.

Technically, the above RUN command could be removed entirely from the Dockerfile and the ci tests would run successfully, but then you would be unable to successfully build the Dockerfile locally on non-ubuntu systems, which I find useful for debugging the CI-tests.

[yantosca]

The check may have failed because probably didn't update the text replacement in the Dockerfile. I can look into a fix.

[yantosca]

Also the Dockerfile now pegs Ubuntu 20.04 and AMD64 platform, so that library paths will be consistent.

[jimmielin]

Thanks @laestrada and @yantosca! I didn't know there was a specific fix in Dockerfile for the library path.

Perhaps we should also improve Makefile.defs's check for FLEX_LIB_DIR. This blurb of code that is currently only for Cannon is probably useful to add:

  ifeq ($(USER_FLEX_LIB_DIR),)
    FLEX_LIB_DIR = ${FLEX_HOME}/lib64
  else
    FLEX_LIB_DIR = ${USER_FLEX_LIB_DIR}
  endif

Maybe we could add this:

  ifneq ($(USER_FLEX_LIB_DIR),)
    FLEX_LIB_DIR = ${USER_FLEX_LIB_DIR}
  endif

And then we could probably just pass USER_FLEX_LIB_DIR=/lib/x86_64-linux-gnu from either Dockerfile or the ci-testing-script, avoiding the text replacement.

[yantosca]

@jimmielin: I was thinking of a better way to specify the FLEX_LIB_DIR. With CMake it'd be easier to write a script to do that but in GNU Make we have to jump thru a few hoops. I'll try to implement your suggestion. Stay tuned.

[laestrada]

@jimmielin good suggestion! It would be much easier to just specify an env variable for FLEX_LIB_DIR in the Dockerfile

[yantosca]

@jimmielin, @RolfSander, this seems to work OK. I've tested it in the container locally (setting an environment variable KPP_FLEX_LIB_DIR=/lib/x86_64-linux-gnu (without doing text replacement).

############################################################
### Default settings                                     ###
############################################################

CC           := gcc
CC_FLAGS     := -g -Wall -Wno-unused-function
FLEX         := flex
FLEX_LIB_DIR := /usr/lib
BISON        := bison
INCLUDE_DIR  := /usr/include
SYSTEM       := $(shell uname)        # returns e.g. "Linux"
SYSTEM_M     := $(shell uname -m)     # returns e.g. "x86_64"
HOST         := $(shell hostname)     # returns name of machine

############################################################
### Keep looking for the flex library until found        ###
### Otherwise use the path specified in KPP_FLEX_LIB_DIR ###
###                                                      ###
### NOTE for MacOS X: IF you have installed flex with    ###
### HomeBrew, then flex will be installed to a path such ### 
### as /usr/local/Cellar/flex/X.Y.Z/lib, where X.Y.Z is  ###
### the version number.  --  Bob Yantosca (01 Nov 2021)  ###   
############################################################

# Try /usr/lib64
ifeq ($(wildcard $(FLEX_LIB_DIR)/libfl.*),)
  FLEX_LIB_DIR := /usr/lib64
endif

# Try /usr/local/lib
ifeq ($(wildcard $(FLEX_LIB_DIR)/libfl.*),)
  FLEX_LIB_DIR := /usr/local/lib
endif

# Try /usr/local/lib64
ifeq ($(wildcard $(FLEX_LIB_DIR)/libfl.*),)
  FLEX_LIB_DIR := /usr/local/lib64
endif

# If flex isn't found, look again e.g. /lib/x86_64
ifeq ($(wildcard $(FLEX_LIB_DIR)/libfl.*),)
  FLEX_LIB_DIR := /lib/$(SYSTEM_M)
endif

# If we can't find the flex library path, then specify it
# from the user's environment variable KPP_FLEX_LIB_DIR.
ifneq ($(KPP_FLEX_LIB_DIR),)
  FLEX_LIB_DIR = ${KPP_FLEX_LIB_DIR}
endif

# ERROR CHECK: EXIT if we can't find libfl.a (Flex library file)
ifeq ($(wildcard $(FLEX_LIB_DIR)/libfl.*),)
 $(error "Could not find the Flex library at $(FLEX_LIB_DIR)! Specify FLEX_LIB_DIR in Makefile.defs.")
endif

############################################################
### System-specific or host-specific default settings    ###
############################################################

# Settings for MacOS
ifeq ($(SYSTEM),Darwin)
  CC_FLAGS += -DMACOS -O
endif

#############################################################

If you think this is OK I can merge & also update the docs.

[yantosca]

Also I think we can change Specify FLEX_LIB_DIR in Makefile.defs." to Specify the path to the flex library in env var KPP_FLEX_LIB_DIR.

[jimmielin]

Hi @yantosca, looks good to me! Thanks for improving this, getting rid of system-specific checks will definitely make Makefile.defs easier to maintain in the future.

[yantosca]

Thanks @jimmielin. Will test on my Mac and also on Cannon then will merge.

[jimmielin]

Hi @yantosca, we might have to add this clause as well:

# If flex isn't found, look again e.g. /usr/lib/$(arch)-linux-gnu
ifeq ($(wildcard $(FLEX_LIB_DIR)/libfl.*),)
  FLEX_LIB_DIR := /usr/lib/$(firstword $(SYSTEM_M))-linux-gnu
endif

This will then look for /usr/lib/x86_64-linux-gnu.

Tested on AWS Arm instances as well:

$ uname -a
Linux ip-172-31-38-162 5.15.0-1009-aws #11-Ubuntu SMP Thu May 26 19:39:49 UTC 2022 aarch64 aarch64 aarch64 GNU/Linux
$  ls /usr/lib/$(uname -m)-linux-gnu | grep libfl.a
libfl.a
$ ls /usr/lib/aarch64-linux-gnu/ | grep libfl.a
libfl.a

Edit: Actually firstword is needed, apparently $(SYSTEM_M) has some trailing spaces

[yantosca]

Thanks, I'll add that too @jimmielin.

[jimmielin]

Sorry, I just realized a weird bug where $(SYSTEM_M) has trailing spaces, so it needs to be wrapped in $(firstword $(SYSTEM_M)). This works for me:

# If flex isn't found, look again e.g. /usr/lib/$(arch)-linux-gnu
ifeq ($(wildcard $(FLEX_LIB_DIR)/libfl.*),)
  FLEX_LIB_DIR := /usr/lib/$(firstword $(SYSTEM_M))-linux-gnu
endif

[yantosca]

Also updated documentation:

• https://kpp.readthedocs.io/en/latest/getting_started/01_installation.html
• https://kpp.readthedocs.io/en/latest/tech_info/06_info_for_kpp_developers.html

[yantosca]

@jimmielin feel free to make any edits to the doc. I think I've covered it though.

[jimmielin]

Thanks @yantosca! The documentation looks great as well!