#1094853 pam: building with or without bison

Package:
src:pam
Source:
src:pam
Submitter:
Helmut Grohne
Date:
2025-02-04 20:54:06 UTC
Severity:
normal
Tags:
#1094853#5
Date:
2025-01-31 19:13:14 UTC
From:
To:
Hi Sam,

I noticed that your pam update fails to build in my cross bootstrap
environment and looked into it. Your switch to meson closed my earlier
FTCBFS bug and I fully concur with such closing, because I see FTCBFS
bugs as actionable patch communication. Since the patch no longer makes
sense, you closed the bug leaving the verification to me. Please
continue using this process for all FTCBFS bugs filed by me.

For digging into the failure, I first tried cross building pam in a
clean environment and it would just work. Adding a stage1 profile
wouldn't make it fail either. The cause must somehow be related to the
bootstrap environment not being very clean and indeed adding a bison
package to the cross build made it fail. This got me looking and I
figured that pam looks for bison and marks this tool as a "disabler".
What is being disabled here is the generation of
/usr/share/doc/libpam-doc/txt/draft-morgan-pam-current.txt.gz. This file
is missing from the current libpam-doc package. Is that intentional? If
it is, I suggest making it explicit by adding Build-Conflicts: bison. If
you'd rather have the file, I suggest adding Build-Depends-Indep: bison.

I still haven't answered what is failing here beyond it somehow being
connected to the presence or absence of bison. The parser generator is
being used to build a tool called "padout" in docs/specs/. This is run
during built, but it is not installed anywhere. As such, it should be
"native: true". Once you add that, a cross build pass with bison
installed also succeeds.

Given that stuff basically works and that I am hitting a strange corner
case by using a dirty build environment, I've set the severity to
wishlist and leave the decision of how to handle it up to you. I hope
what I've written here makes sense to you.

Helmut

#1094853#12
Date:
2025-02-01 19:01:39 UTC
From:
To:
    Helmut> What is being disabled here is the generation of
    Helmut> /usr/share/doc/libpam-doc/txt/draft-morgan-pam-current.txt.gz. This
    Helmut> file is missing from the current libpam-doc package. Is that
    Helmut> intentional?

No.
I had entirely missed this particular bit of pam trivia.

    Helmut> adding Build-Conflicts: bison. If you'd rather have the
    Helmut> file, I suggest adding Build-Depends-Indep: bison.

Is that sufficient?

You also talked about adding native:true for bison. But Is that sufficient?
Then don't I need to use a native flex and a native C compiler?
I don't really understand how all that works in meson, but it seems like
I'd need all native tools to build the spec.
I guess we only will consume the spec on the arch all build,  and
there's no reason for arch all to ever cross build I can think of.
But it seems like we also need to make sure the arch cross builds
succeed.
I don't understand how adding build-indep on bison actually fixes the
problem.  I agree it makes it less likely because it means I now need a
native bison in my build environment rather than a cross bison to
trigger things.
But if I just make bison (or byacc which is also supported) native,
won't I try to build using a cross flex, cross compiler and native
bison?

I don't understand how much of the compiler discovery and argument
detection I would need to replicate to build both cross and native tools.

I wonder if a simpler solution would be to only build the spec if we're
building docs.
So:

* add bison as build-indep
* Only build the spec if the docs feature is enabled.

#1094853#17
Date:
2025-02-01 19:01:39 UTC
From:
To:
    Helmut> What is being disabled here is the generation of
    Helmut> /usr/share/doc/libpam-doc/txt/draft-morgan-pam-current.txt.gz. This
    Helmut> file is missing from the current libpam-doc package. Is that
    Helmut> intentional?

No.
I had entirely missed this particular bit of pam trivia.

    Helmut> adding Build-Conflicts: bison. If you'd rather have the
    Helmut> file, I suggest adding Build-Depends-Indep: bison.

Is that sufficient?

You also talked about adding native:true for bison. But Is that sufficient?
Then don't I need to use a native flex and a native C compiler?
I don't really understand how all that works in meson, but it seems like
I'd need all native tools to build the spec.
I guess we only will consume the spec on the arch all build,  and
there's no reason for arch all to ever cross build I can think of.
But it seems like we also need to make sure the arch cross builds
succeed.
I don't understand how adding build-indep on bison actually fixes the
problem.  I agree it makes it less likely because it means I now need a
native bison in my build environment rather than a cross bison to
trigger things.
But if I just make bison (or byacc which is also supported) native,
won't I try to build using a cross flex, cross compiler and native
bison?

I don't understand how much of the compiler discovery and argument
detection I would need to replicate to build both cross and native tools.

I wonder if a simpler solution would be to only build the spec if we're
building docs.
So:

* add bison as build-indep
* Only build the spec if the docs feature is enabled.

#1094853#22
Date:
2025-02-01 20:52:21 UTC
From:
To:
Hi Sam,
"mostly". It still fails at is cross building pam in a non-minimal (i.e.
unsupported) build environment that happens to contain bison.

Sufficient for what? Adding it makes the non-minimal cross build stop
failing.

flex comes in two parts one is the flex tool (where there is only one)
and the other is libfl-dev containing libfl.so (where you already
build-depend on both). A native C compiler is implied in build-essential
(and crossbuild-essential-*).

It is assembling an executable() from source files there. While that
results in multiple compiler invocations and a linker invocation, you
only need to specify "native: true" once here. Meson usually is clever
enough to not mix build and host objects.

I confirm in a Debian context. Outside Debian, people might want to
cross build pam and build its documentation in one go.

Yes. It already does succeed if your build environment is minimal and
thus does not contain bison. If it contains bison (which it should not
do), it presently fails.
needs to be fixed by having a reasonable build environment. What it does
is building that additional documentation file.

There is no difference between native and cross bison. Bison translates
one textual format to another and it thus is marked M-A:foreign.

There also is no cross flex as it is in the same spot as bison except
for also providing a library libfl.so which has been split out of flex
into libfl-dev to allow flex to be M-A:foreign.
cross tools and will automatically detect them. It may need a native
libfl.so (but you already depend on that) and flex and bison are
M-A:foreign tools such that there is no cross/native distinction there.
The addition of "native: true" is something I expect upstream to accept
as a generally useful patch and not a Debian-specific workaround
independent of how we solve the other aspects.

I very much welcome not building docs in the arch-only build. It removes
things that may fail from the picture and it also becomes slightly
faster.

I guess the latter is a matter of adding

    -Ddocs=$(if $(filter libpam-doc,$(shell dh_listpackages)),on,off)

to meson_OPTS.

That is solving even more things and it makes the proposed addition of
"native: true" irrelevant to the Debian package, but I suggest still
forwarding it upstream as it serves as documentation for an intention
and it may help other cross distributions such as Yocto.

I hope that this mail answers more questions than it leaves.

Helmut

#1094853#27
Date:
2025-02-03 08:16:26 UTC
From:
To:
    >> You also talked about adding native:true for bison. But Is that
    >> sufficient?

    Helmut> Sufficient for what? Adding it makes the non-minimal cross
    Helmut> build stop failing.

Well, makes your particular case stop failing.
I guess I was asking whether adding native=true to finding bison is
sufficient to get the spec built if a native bison is present.

    >> Then don't I need to use a native flex and a native C compiler?

    Helmut> flex comes in two parts one is the flex tool (where there is
    Helmut> only one) and the other is libfl-dev containing libfl.so
    Helmut> (where you already build-depend on both). A native C
    Helmut> compiler is implied in build-essential (and
    Helmut> crossbuild-essential-*).

I was more thinking about the Meson side of things rather than the
Debian side of things.
Do I need to duplicate the logic to choose compiler flags and to set up
config.h for the cross side as well as the native side?
There's a lot of logic in the top level meson.build for selecting
compiler and compiler options (and generating config.h).
I suspect in most cases it will probably just work to use the same
options for cross and native, but we've certainly come across cases
where that's not true (for example the ftcbfs patch against pam was such
a case).
I don't know how all that works in Meson.

For example we explicitly find a compiler in the top level meson.build,
although only use that meson object for autoconf-style probing of the system.
We never tell it to use that compiler for building executables.
So there's a lot going on implicitly.

#1094853#32
Date:
2025-02-03 09:57:11 UTC
From:
To:
Hi Sam,

What you write makes me guess that there still is a misunderstanding.

Qualifying bison as "native" does not make sense. It translates one
textual format (yacc grammar) to another textual format (C source). Its
behaviour is not dependent on the architecture.

The next part is that the native=true I propose to add has nothing to do
with bison. It applies to the compiler being used to compile the sources
generated by bison

I confirm that with the proposed patch and bison installed, the spec is
produced both in native and in cross builds.

I suspect that you believe that the line

    cc = meson.get_compiler('c')

chooses a compiler. This is not the case. The compiler is automatically
chosen whether you ask meson for the compiler object or not. The same
holds for (some of the) compiler flags. When adding native=true to an
executable(), meson will implicitly call meson.get_compiler() and
forward the native flag and thus this all just works.

Whether you need a config.h depends on whether you use it in the sources
being compiled. The relevant .y and .l files do #include <config.h>, so
in principle you do, because the contents of config.h typically are
architecture-dependent. Practically speaking, it does work without
adding supplying one. I attempted a rebuild with those #includes dropped
and it just worked. So yeah, it's better to think about config.h
explicitly and I suggest dropping it here. Updated patch attached.

That's only partially true. The project() declaration influences the
compiler selection and default options. These apply to both native and
cross compilers. The config.h is generated in libpam/include/meson.build
rather than the top level file from the configuration_data() object
cdata defined in the top level. Since it also has a number of HAVE_*
macros, it definitely is architecture-dependent.

Agreed.

What you call explicit, I call implicit, as we merely tell it that this
is a c project with some default options. But then we also implicitly
find a native compiler that way!

Exactly! Note that add_project_*() also has a native parameter that
defaults to false, so all of those add_project_*() in meson.build only
modify the regular compiler (cross compiler when doing a cross build)
and do not influence the native compiler (when they differ). In
principle, you might argue that try_cc_flags would also reasonably apply
to the native compiler and we should add:

    add_project_arguments(
      meson.get_compiler('c', native: true).get_supported_arguments(try_cc_flags),
      language: 'c',
      native: true)

I appreciate your attention to detail. I concur that just adding
"native: true" to the one executable() may work now and produce subtle
breakage later. I argue that such breakage is limited to situations that
are broken due to the lack of "native: true" right now.

Helmut