Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Creates a standard autotools setup. #89

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ceneblock
Copy link

@ceneblock ceneblock commented Jan 8, 2018

I wasn't a fan of the static Makefile, this will turn the project into using autotools which detects missing libs and sets everything "auto-magically" for the end user.

It also makes life easier for the maintainers.

@spth
Copy link
Collaborator

spth commented Jan 19, 2018

While I can see the advantage of using autotools to detect libusb, using autotools also adds some overhead. It makes it harder for users to compile the program (especially since configure is not in the repository).

Also, why do you use all those

AC_CHECK_HEADER_STDBOOL
AC_TYPE_SIZE_T
AC_TYPE_SSIZE_T
AC_TYPE_UINT16_T
AC_TYPE_UINT32_T
AC_TYPE_UINT8_T

instead of just requiring a C99-compliant C compiler?

Also in these checks:

AC_FUNC_MALLOC
AC_CHECK_FUNCS([memset strcasecmp strdup strerror strrchr])

The only ones that make sense to me are strcasecmp and strdup, as the other ones are standard C functions available at least on any hosted C implementation.

And for

AC_CHECK_HEADERS([fcntl.h malloc.h stddef.h stdint.h stdlib.h string.h strings.h sys/param.h termios.h unistd.h])

malloc.h is no longer used in stm8flash, and again many of the headers are standard C headers.

Philipp

@ceneblock
Copy link
Author

My apologies for not including the configure file. I can get that in there no problem

In regards to the info in configure.ac those were all pulled in from autoscan. If I understand correctly, the line "AC_PROG_CC_STDC" in configure.ac should ensure that C99 is present (at this current time); however, I can add additional checks to verify that C99 is indeed present.

I can see an argument both ways for just including a Makefile and having the configure; however, the configure ensures that all libraries are present and accessible while providing additional output if they are not. This also follows the "standard" for GNU packages, in addition to the INSTALL file explaining how to properly compile the program.

Thank you for taking the time to respond to me!

@spth
Copy link
Collaborator

spth commented Jan 20, 2018

AC_PROG_CC_STDC will AFAIk still fall back to a C80/C90 compiler. I am not an autotools expert, but I think AC_PROG_CC_C99 and checking ac_cv_prog_cc_c99 is still needed.

Do we really want a separate include directory? Since we are just building an executable, and no libraries, IMO, the he4ader files could just go into src.

Philipp

@ceneblock
Copy link
Author

From https://www.gnu.org/software/autoconf/manual/autoconf-2.60/html_node/C-Compiler.html:

If the C compiler cannot compile ISO Standard C (currently C99), try to add an option to output variable CC to make it work. If the compiler does not support C99, fall back to supporting ANSI C89 (ISO C90).

After calling this macro you can check whether the C compiler has been set to accept Standard C; if not, the shell variable ac_cv_prog_cc_stdc is set to ‘no’.

So, you're correct. I'll make the changes.

Looking at https://ftp.gnu.org/gnu/hello/hello-2.10.tar.gz, they have header files, but do not have a separate include/. That said, I think it looks cleaner, but it is up to the project on that one. I'll do whatever is needed.

@spth
Copy link
Collaborator

spth commented Sep 9, 2021

Any news on this one?

@kasunch
Copy link

kasunch commented Nov 2, 2021

Just out of curiosity, why don’t you use a modern build system generator like CMake?

@ceneblock
Copy link
Author

Just out of curiosity, why don’t you use a modern build system generator like CMake?

Because I don't know CMake. :)

Everything I do is on Linux or FreeBSD so the GNU build system works perfectly for me.

CMake is probably a better choice. If you'd like to do that then I'll close my PR. Looks like I need to fix this anyways, although a 4 year old PR pretty much tells me that this won't get fixed.

@spth
Copy link
Collaborator

spth commented Nov 3, 2021

Well, I don't know cmake either. My experience with autotools is limited, but I'd still feel more comfortable reviewing a autotools pull request than a cmake one.

My comment above is partially obsolete now, as AFAIK AC_PROG_CC_C99 is now deprecated.

I don't have experience with building stm8flash on systems other than Unices, and wonder how the autotools setup will affect building for Windows.

@kasunch
Copy link

kasunch commented Nov 12, 2021

The Makefile used at the moment is uncomplicated.
So writing a CMakeLists.txt file would be straightforward.
I have a fair share of experience with Cmake, so I can contribute to this.

After looking at the Makefile, the question I have is if moving to Autotools or CMake or anything else is worthy at the moment.
As I see, the only dependency is libusb.
Use of the of pkg-config in the Makefile is probably fine and simple for the moment.

If moving to a build system generator is worthy, I suggested CMake since it is modern and has a wider support for dependency resolving.
However, I don’t have a strong opinion on this over Autotools.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants