Skip to content

Conversation

@richi235
Copy link

gcc gave me the following warning:

lauxlib.c: In function ‘luaL_loadfile’:
lauxlib.c:577:4: warning: this ‘while’ clause does not guard... [-Wmisleading-indentation]
  577 |    while ((c = getc(lf.f)) != EOF && c != LUA_SIGNATURE[0]) ;
      |    ^~~~~
lauxlib.c:578:5: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the ‘while’
  578 |     lf.extraline = 0;
      |     ^~

Since this was a small thing I decided to fix it quickly. This might be fixed
already in upstream lua, in that case, feel free to ignore :)

gcc gave me the following warning:

lauxlib.c: In function ‘luaL_loadfile’:
lauxlib.c:577:4: warning: this ‘while’ clause does not guard... [-Wmisleading-indentation]
  577 |    while ((c = getc(lf.f)) != EOF && c != LUA_SIGNATURE[0]) ;
      |    ^~~~~
lauxlib.c:578:5: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the ‘while’
  578 |     lf.extraline = 0;
      |     ^~

Since this was a small thing I decided to fix it quickly. This might be fixed
already in upstream lua, in that case, feel free to ignore :)
@hartwork
Copy link

hartwork commented Mar 17, 2025

Hello @richi235,

I've seen the warning too, I agree it's a bit annoying 😃

From my point of view, a bundled dependency ideally stays as close to what upstream does and arguably the sustainable way to fix compile warnings like these would be to fix them upstream for everyone and then to forward the bundled copy to a version with the fix downstream.

Lua upstream has fixed this very warning (with a slightly different approach) in commit lua/lua@f858a15 that is included with Lua >=5.2.0 (if we ignore pre-releases).

There are potentially all these options to consider moving forward:

  • a) Updating the bundled Lua to latest 5.4.7
  • b) Updating the bundled Lua to only 5.2.0
  • c) Applying your fix (or cherry-pick from upstream)
  • d) Adjusting the build system to build Lua without (this or any) warnings
  • e) Tolerate warnings from bundled dependencies (i.e. no changes)
  • f) (unbundle Lua)

One key question to answer in my eyes is whether e2factory has any reasons to stick to Lua 5.1.x for compatibility to something else, because if so that would kill options (a) and (b) — any ideas?

If (a) is possible and feasible, that would be my favorite. There is no guarantee it would be free of new/other warnings before trying, though.

The fact that C compilers — both GCC and Clang — get stricter by the minute with recent releases would be a reason to consider (e).

Best, Sebastian

@tuemlix
Copy link
Member

tuemlix commented Mar 20, 2025

There is a concern that updating Lua could change build behavior, so that was never done. But I agree this can't continue forever.

Obviously patching upstream is more of a last resort thing. A Lua update would be preferable. There are also a couple of extensions that would need to be gone through at the same time.

Switching to LuaJIT would be my favorite solution. There is noticeable lag loading large projects, calculating dependencies, checksums etc. and that would help a little.

In any case that's not a project I can take on currently, as you may have guessed from the dormant state of this repo.

@hartwork
Copy link

@tuemlix thanks for your reply!

Whenever you find time again — not urgent —, I would want to ask back:

  • I don't fully understand the "could change build behavior" part. Change in what way?
  • The "couple of extensions that would need to be gone through" is also something I would like to understand better. Extensions where? In-house or at customers or …? (My guess is that we are talking about
    SUBDIRS = $(shell find . -mindepth 1 -maxdepth 1 -type d)
    here.)

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