Re: Changes in HAProxy 3.0's Makefile and build options

2024-04-14 Thread Илья Шипицин
сб, 13 апр. 2024 г. в 15:26, Willy Tarreau :

> Hi Tristan,
>
> On Fri, Apr 12, 2024 at 07:38:18AM +, Tristan wrote:
> > Hi Willy,
> >
> > > On 11 Apr 2024, at 18:18, Willy Tarreau  wrote:
> > >
> > > Some distros simply found that stuffing their regular CFLAGS into
> > > DEBUG_CFLAGS or CPU_CFLAGS does the trick most of the time. Others use
> > > other combinations depending on the tricks they figured.
> >
> > Good to know I wasn't alone scratching my head about what came from
> where!
>
> Definitely.
>
> > > After this analysis, I figured that we needed to simplify all this, get
> > > back to what is more natural to use or more commonly found, without
> > > breaking everything for everyone. [...]
> >
> > These are very nice improvements indeed, but I admit that if (at least
> > partially) breaking backwards compatibility was the acceptable choice
> here,
>
> It should almost not break it otherwise it will indicate what changed
> and how to proceed.
>
> > I'd have hoped to see something like cmake rather than a makefile
> > refactoring.
>

I would like to hear real pain behind the idea of moving to cmake.


I suspect such pain exists, for example I had to explicitly specify LUA
options depending on linux distro (it was hard to autodetect
LUA by Makefile, but it is easier for cmake)

maybe we can deal with the pain and improve things using make :)



>
> That's orthogonal. cmake is an alternative to autoconf, not to make,
> you still run "make -j$(nproc)" once cmake is done.
>
> And such mechanisms are really really a pain to deal with, at every
> stage (development and user). They can be justified for ultra-complex
> projects but quite frankly, having to imagine not being able to flip
> an option without rebuilding everything, not having something as simple
> as "V=1" to re-run the failed file and see exactly what was tried,
> having to fight against the config generator all the time etc is really
> a no-go for me. I even remember having stopped using OpenCV long ago
> when it switched from autoconf to cmake because it turned something
> complicated to something out of control that would no longer ever build
> outside of the authors' environment. We could say whatever, like they
> did it wrong or anything else, but the fact is I'm not going to impose
> a similar pain to our users.
>
> In our case, we only need to set a list of files to be built, and pass
> a few -I/-L/-l while leaving the final choice to the user.
>
> > Actually, I'd thought a few times in the past about starting a
> discussion in
> > that direction but figured it would be inconceivable.
> >
> > I don't know how controversial it is, so the main two reasons I mention
> it are:
> > - generally better tooling (and IDE support) out of the box:
> options/flags
> >   discovery and override specifically tends to be quite a bit simpler as
> the
> >   boilerplate and conventions are mostly handled by default
> > - easier to parse final result: both of them look frankly awful, but the
> >   result of cmake setups is often a little easier to parse as it
> encourages a
> >   rather declarative style (can be done in gmake, but it is very much up
> to the
> >   maintainers to be extremely disciplined about it)
>
> The problem with tools like cmake is not to parse the output when it
> works but to figure how to force it to accept that *you* are the one who
> knows when it decides it will not do what you want. While a makefile can
> trivially be overridden or even fixed, good luck for guessing all the
> magic names of cmake variables that are missing and that may help you.
>
> I really do understand why some super complex projects involving git
> submodules and stuff like this would go in that direction, but otherwise
> it's just asking for trouble with little to no benefit.
>
> > Arguably, there's the downside of requiring an extra tool everyone might
> not
> > be deeply familiar with already, and cmake versions matter more than
> gmake
> > ones so I would worry about compatibility for distros of the GCC 4 era,
> but
> > it seemed to me like it's reasonably proven and spread by now to
> consider.
>
> It's not even a matter of compatibility with any gcc version, rather a
> compatibility with what developers are able to write and what users want
> to do if that was not initially imagined by the developers. Have you read
> already about all the horrors faced by users trying to use distcc or ccache
> with cmake, often having to run sed over their cmake files ? Some time ago,
> cmake even implemented yet another magic variable specifically to make this
> less painful. And cross-compilation is yet another entire topic. All of
> this just ends up in a situation where the build system becomes an entire
> project on its own, just for the sake of passing 6 variables to make in
> the end :-/  In the case of a regular makefile at least, 100% of the
> variables you may have to use are present in the makefile, you don't need
> to resort to random guesses by 

[PATCH 2/2] CLEANUP: assorted typo fixes in the code and comments

2024-04-14 Thread Ilya Shipitsin
This is 41st iteration of typo fixes
---
 doc/configuration.txt  | 4 ++--
 include/haproxy/cli-t.h| 2 +-
 include/haproxy/session.h  | 2 +-
 include/haproxy/vecpair.h  | 2 +-
 reg-tests/ssl/ocsp_auto_update.vtc | 2 +-
 src/haproxy.c  | 2 +-
 src/linuxcap.c | 2 +-
 src/log.c  | 2 +-
 src/ring.c | 2 +-
 9 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 51aefb1fa..bc7f5c1c4 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -6733,7 +6733,7 @@ fullconn 
 
 guid 
   Specify a case-sensitive global unique ID for this proxy. This must be unique
-  accross all haproxy configuration on every object types. Format is left
+  across all haproxy configuration on every object types. Format is left
   unspecified to allow the user to select its naming policy. The only
   restriction is its length which cannot be greater than 127 characters. All
   alphanumerical values and '.', ':', '-' and '_' characters are valid.
@@ -16994,7 +16994,7 @@ force-tlsv13
 
 guid 
   Specify a case-sensitive global unique ID for this server. This must be
-  unique accross all haproxy configuration on every object types. See "guid"
+  unique across all haproxy configuration on every object types. See "guid"
   proxy keyword description for more information on its format.
 
 id 
diff --git a/include/haproxy/cli-t.h b/include/haproxy/cli-t.h
index 6e0abae57..8555ea8c7 100644
--- a/include/haproxy/cli-t.h
+++ b/include/haproxy/cli-t.h
@@ -56,7 +56,7 @@ enum {
CLI_ST_INIT = 0,   /* initial state, must leave to zero ! */
CLI_ST_END,/* final state, let's close */
CLI_ST_GETREQ, /* wait for a request */
-   CLI_ST_PARSEREQ,   /* pase a request */
+   CLI_ST_PARSEREQ,   /* parse a request */
CLI_ST_OUTPUT, /* all states after this one are responses */
CLI_ST_PROMPT, /* display the prompt (first output, same code) */
CLI_ST_PRINT,  /* display const message in cli->msg */
diff --git a/include/haproxy/session.h b/include/haproxy/session.h
index b70a5a89c..a9cea62ed 100644
--- a/include/haproxy/session.h
+++ b/include/haproxy/session.h
@@ -209,7 +209,7 @@ static inline int session_add_conn(struct session *sess, 
struct connection *conn
}
LIST_APPEND(>conn_list, >sess_el);
 
-   /* Ensure owner is set for connection. It could have been resetted
+   /* Ensure owner is set for connection. It could have been reset
 * prior on after a session_add_conn() failure.
 */
conn->owner = sess;
diff --git a/include/haproxy/vecpair.h b/include/haproxy/vecpair.h
index f5337af6a..e49570636 100644
--- a/include/haproxy/vecpair.h
+++ b/include/haproxy/vecpair.h
@@ -436,7 +436,7 @@ static inline size_t vp_get_varint_ofs(struct ist *v1, 
struct ist *v2, size_t of
 
vp_skip(v1, v2, ofs);
 
-   /* let's see where we start from. The wraping area only concerns the
+   /* let's see where we start from. The wrapping area only concerns the
 * end of the first area, even if it's empty it does not overlap with
 * the second one so we don't care about v1 being set or not.
 */
diff --git a/reg-tests/ssl/ocsp_auto_update.vtc 
b/reg-tests/ssl/ocsp_auto_update.vtc
index e3a7ae6ae..8bb138bc6 100644
--- a/reg-tests/ssl/ocsp_auto_update.vtc
+++ b/reg-tests/ssl/ocsp_auto_update.vtc
@@ -645,7 +645,7 @@ process p7 -wait
 
 ##
 ##
-# EIGTH TEST CASE#
+# EIGHTH TEST CASE   #
 ##
 ##
 
diff --git a/src/haproxy.c b/src/haproxy.c
index 208c9bccb..be8b587cc 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -3490,7 +3490,7 @@ int main(int argc, char **argv)
 #if defined(USE_LINUX_CAP)
/* If CAP_NET_BIND_SERVICE is in binary file permitted set and process
 * is started and run under the same non-root user, this allows
-* binding to priviledged ports.
+* binding to privileged ports.
 */
prepare_caps_from_permitted_set(geteuid(), global.uid, argv[0]);
 #endif
diff --git a/src/linuxcap.c b/src/linuxcap.c
index 7058370de..b330296a8 100644
--- a/src/linuxcap.c
+++ b/src/linuxcap.c
@@ -71,7 +71,7 @@ static uint32_t caplist;
  * will be unset by the same reason.
  * We do this only if the current euid is non-root and there is no global.uid.
  * Otherwise the process will continue either to run under root, or it will do
- * a transition to unpriviledged user later in prepare_caps_for_setuid(),
+ * a transition to unprivileged user later in prepare_caps_for_setuid(),
  * which specially manages its capabilities in that case.
  * Always returns 0. Diagnostic warnings will be emitted only, if
  * LSTCHK_NETADM is presented in LSTCHK_NETADM and some failures are
diff --git a/src/log.c b/src/log.c
index 

[PATCH 1/2] CI: reduce ASAN log redirection umbrella size

2024-04-14 Thread Ilya Shipitsin
previously ASAN_OPTIONS=log_path=asan.log was intended for VTest
execution only, it should not affect "haproxy -vv" and hsproxy
config smoke testing
---
 .github/workflows/vtest.yml | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/.github/workflows/vtest.yml b/.github/workflows/vtest.yml
index 9d0bf48b0..5ee8a7a64 100644
--- a/.github/workflows/vtest.yml
+++ b/.github/workflows/vtest.yml
@@ -42,8 +42,6 @@ jobs:
   # Configure a short TMPDIR to prevent failures due to long unix socket
   # paths.
   TMPDIR: /tmp
-  # Force ASAN output into asan.log to make the output more readable.
-  ASAN_OPTIONS: log_path=asan.log
   OT_CPP_VERSION: 1.6.0
 steps:
 - uses: actions/checkout@v4
@@ -143,6 +141,9 @@ jobs:
   run: echo "::add-matcher::.github/vtest.json"
 - name: Run VTest for HAProxy ${{ steps.show-version.outputs.version }}
   id: vtest
+  env:
+# Force ASAN output into asan.log to make the output more readable.
+ASAN_OPTIONS: log_path=asan.log
   run: |
 # This is required for macOS which does not actually allow to increase
 # the '-n' soft limit to the hard limit, thus failing to run.
-- 
2.44.0




[PATCH 0/2] CI cleanup, spell fixes

2024-04-14 Thread Ilya Shipitsin
the main part is reducing ASAN_OPTIONS scope, it was supposed
only to capture output of vtests, accidently it covered "config smoke tests" as 
well

Ilya Shipitsin (2):
  CI: reduce ASAN log redirection umbrella size
  CLEANUP: assorted typo fixes in the code and comments

 .github/workflows/vtest.yml| 5 +++--
 doc/configuration.txt  | 4 ++--
 include/haproxy/cli-t.h| 2 +-
 include/haproxy/session.h  | 2 +-
 include/haproxy/vecpair.h  | 2 +-
 reg-tests/ssl/ocsp_auto_update.vtc | 2 +-
 src/haproxy.c  | 2 +-
 src/linuxcap.c | 2 +-
 src/log.c  | 2 +-
 src/ring.c | 2 +-
 10 files changed, 13 insertions(+), 12 deletions(-)

-- 
2.44.0