search

iraf-community / x11iraf

X11/GUI utilities and applications for IRAF (xgterm, ximtool, xtapemon)
https://iraf-community.github.io/x11iraf
21 stars 8 forks source link

Fix tcl.h path error on some Linux distribution #32

Closed JiahuiZhuo closed 3 years ago

JiahuiZhuo commented 3 years ago

Fix the compilation error happened on in #31

olebole commented 3 years ago

Thank you very much for your patch, which seems to work!

I would however think that it is unnecessarily complicated (and prone to errors) to have source code patched during the build process. Instead of patching the source code during the build, it may be better to change these lines unconditionally in the patch, and to add a -I${TCL_INCLUDE_DIR} to the compilation flags. This variable could then just be set to /usr/include/tcl in the Makefile, and could be overwritten from the command line if needed. Arch Linux would not need to use it, but it doesn't hurt if it is set there.

JiahuiZhuo commented 3 years ago

Thank you very much for your patch, which seems to work!

I would however think that it is unnecessarily complicated (and prone to errors) to have source code patched during the build process. Instead of patching the source code during the build, it may be better to change these lines unconditionally in the patch, and to add a -I${TCL_INCLUDE_DIR} to the compilation flags. This variable could then just be set to /usr/include/tcl in the Makefile, and could be overwritten from the command line if needed. Arch Linux would not need to use it, but it doesn't hurt if it is set there.

Great idea! I also think that using sed in Makefile is quite dangerous. I'm going to update the pull.

olebole commented 3 years ago

Thank you very much! I will merge this.