Opened 5 years ago

Closed 5 years ago

#3723 closed enhancement (fixed)

cross compile KEA

Reported by: fdupont Owned by: fdupont
Priority: low Milestone: Kea0.9.1
Component: Unclassified Version: git
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: Low
Sub-Project: DHCP Feature Depending on Ticket:
Estimated Difficulty: 0 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description

Fast check on configure.ac & co to find issues:

  • marginal call of AC_TRY_RUN
  • code which should use $host:
    AC_MSG_CHECKING(OS family)
    system=`uname -s`
    

Note IMHO $host is not correctly used in configure.ac...

Subtickets

Change History (14)

comment:1 Changed 5 years ago by fdupont

  • Owner set to fdupont
  • Status changed from new to accepted
  • Type changed from defect to enhancement

comment:2 Changed 5 years ago by fdupont

Moved the AC_TRY_RUN to AC_TRY_COMPILE. Changed system=uname -s for $host and expanded the cases.
Will be available in the trac3723 branch when I'll be back to home.

comment:3 Changed 5 years ago by fdupont

  • Owner changed from fdupont to UnAssigned
  • Status changed from accepted to reviewing

Ready for review.

comment:4 Changed 5 years ago by hschempf

  • Milestone changed from Kea-proposed to Kea0.9.1
  • Owner changed from UnAssigned to sar

comment:5 follow-up: Changed 5 years ago by sar

  • Owner changed from sar to fdupont

The code looks correct but I have two requests.

1) In the AC_TRY_COMPILE please add AC_MSG_CHECKING to indicate what we are doing and AC_MSG_RESULT to indicate how it went.

2) In the cases for the os family is it possible to preset all of the macros to false and then only set the true ones in the switch clauses? It seems like the current set up will be harder to maintain if other OSes are added. So something like this (but with all of the conditionals
and cases):

AC_MSG_CHECKING(OS family)
AM_CONDITIONAL(OS_LINUX, true)
AM_CONDITIONAL(OS_BSD, false)
AM_CONDITIONAL(OS_SOLARIS, false)
case $host in
    *-linux*)
      AM_CONDITIONAL(OS_LINUX, true)
      AC_DEFINE([OS_LINUX], [1], [Running on Linux?])
      OS_TYPE="Linux"
      CPPFLAGS="$CPPFLAGS -DOS_LINUX"
      ;;

comment:6 in reply to: ↑ 5 Changed 5 years ago by fdupont

Replying to sar:

The code looks correct but I have two requests.

1) In the AC_TRY_COMPILE please add AC_MSG_CHECKING to indicate what we are doing and AC_MSG_RESULT to indicate how it went.

=> even the original AC_TRY_RUN was without any message it is a reasonable request.

2) In the cases for the os family is it possible to preset all of the macros to false and then only set the true ones in the switch clauses? It seems like the current set up will be harder to maintain if other OSes are added. So something like this (but with all of the conditionals
and cases):

AC_MSG_CHECKING(OS family)
AM_CONDITIONAL(OS_LINUX, true)
AM_CONDITIONAL(OS_BSD, false)
AM_CONDITIONAL(OS_SOLARIS, false)
case $host in
    *-linux*)
      AM_CONDITIONAL(OS_LINUX, true)
      AC_DEFINE([OS_LINUX], [1], [Running on Linux?])
      OS_TYPE="Linux"
      CPPFLAGS="$CPPFLAGS -DOS_LINUX"
      ;;

=> I'll step back on AM_CONDITIONAL because the doc requires they are called outside if/switch/etc.

comment:7 Changed 5 years ago by fdupont

  • Owner changed from fdupont to sar

All comments (including offline comments) addressed.

comment:8 Changed 5 years ago by sar

  • Owner changed from sar to fdupont

The changes look fine to me. I built and ran make check on my mac and everything seems to be happy.

I believe it can be merged, I don't think it needs a changeling entry.

comment:9 Changed 5 years ago by fdupont

Merged without ChangeLog? entry.

comment:10 Changed 5 years ago by fdupont

  • Resolution set to complete
  • Status changed from reviewing to closed

comment:11 follow-up: Changed 5 years ago by wlodekwencel

  • Resolution complete deleted
  • Status changed from closed to reopened

Reopening because after merging this I'm getting:

checking OS type... Linux
./configure: line 16762: test: =: unary operator expected
./configure: line 16770: test: =: unary operator expected
./configure: line 16778: test: =: unary operator expected
./configure: line 16786: test: =: unary operator expected

on multiple OS (only linux running bash):
Ubuntu 14.04 64b with GNU bash, version 4.3.11(1)-release
CentOS 5 64b with GNU bash, version 3.2.25(1)-release jenkins-output1
Fedora 20 32b with GNU bash, version 4.2.45(1)-release jenkins-output2

comment:12 in reply to: ↑ 11 Changed 5 years ago by fdupont

Replying to wlodekwencel:

Reopening because after merging this I'm getting:

checking OS type... Linux
./configure: line 16762: test: =: unary operator expected
./configure: line 16770: test: =: unary operator expected
./configure: line 16778: test: =: unary operator expected
./configure: line 16786: test: =: unary operator expected

on multiple OS (only linux running bash):
Ubuntu 14.04 64b with GNU bash, version 4.3.11(1)-release
CentOS 5 64b with GNU bash, version 3.2.25(1)-release jenkins-output1
Fedora 20 32b with GNU bash, version 4.2.45(1)-release jenkins-output2

=> should not happen as the tests are the same (i.e., bugs in a priori unmodified part...)

comment:13 Changed 5 years ago by fdupont

Found, I fix it directly in the master.

comment:14 Changed 5 years ago by fdupont

  • Resolution set to fixed
  • Status changed from reopened to closed
Note: See TracTickets for help on using tickets.