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

Issues found in the new build scripts #68

Open
mikrosk opened this issue May 21, 2024 · 21 comments
Open

Issues found in the new build scripts #68

mikrosk opened this issue May 21, 2024 · 21 comments

Comments

@mikrosk
Copy link
Member

mikrosk commented May 21, 2024

To make it easier to find than buried in an email thread, this is the list of things which I have found while testing the new builds.

  • libraries are now (partially) stripped while building

This is wrong. Build process should build stuff and the developer should decide what to do next. install-sh and mintlib.spec (as well as our Github scripts) handle that on their own, exactly as they are supposed to do.

  • DESTDIR vs. prefix: why is the former useful? And even used in the build scripts now?

Traditionally, all "adopted" libraries (gemlib, cflib, etc) use PREFIX for placing include and lib folders. mintlib always has been special, using prefix (lowercase). DESTDIR was never publicly accessible/usable and for some reason it is now. Just for comparison:

make install DESTDIR=<prefix> output:

sbin/tzinit
usr/include
usr/lib
usr/sbin/tzselect, zdump, zic
usr/share

make install prefix=<prefix> output:

include
lib
sbin/tzinit, tzselect, zdump, zic
share

Why forcing the usr folder?

Or, to be precise, it doesn't solve it nicely. As far as I can see, there are explicit steps in the Github script to generate:

mintlib-0.60.1-a50-mintelf.tar.bz2
mintlib-0.60.1-a50-mint.tar.bz2 

These contain the complete mintlib build and tzinit & friends for 68000 and these archives are linked also from https://github.com/freemint/mintlib/blob/master/README.md. Then there's generated:

mintlib-0.60.1-a50-v4e.tar.bz2
mintlib-0.60.1-a50-020.tar.bz2
mintlib-0.60.1-a50-000.tar.bz2

which contain only tzinit & friends for the given architecture. I suppose the idea is to have those three archives downloadable & installable for the freemint snapshots (freemint/freemint#197) but I don't find this easy to understand. At minimum, we should provide:

  1. make & make install which just install the usual files without any executables (so the main archive wouldn't contain any either)
  2. (top-level) make bin & make bin-install (or something like that) which builds the executables (perhaps instead of the type=<target> something more generic as cflags=-cpu_option could be used to make it explicit what is happening)
  3. rename those bin packages to something like mintlib-bin-0.60.1-a50-000.tar.bz2 and link them explicitly (e.g. as -latest) in the README
  • build folder doesn't contain all generated files, not even all object files

I have already mentioned in the email my dislike of the asymmetrical nature of the generated output tree (build/m68000 with directories like argp/.deps, common/.deps, ...). But the asymmetry goes even further, despite the fact that build/m68000 contains sunrpc, syscall and tz folders, object files are still generated in the source directory for those folders what is really ironic because it is -m68000 object files which are generated there.

I am aware that this is a leftover from the previous build system (where the library stuff was generated in lib, lib020 etc but object files for executables were produced in the respective folders) however I fail to see what is the advantage of the new structure then? A lot of changes pushed just to have some files built in another subfolder?


TL; DR version of this is that there are two regressions (stripping & usage of DESTDIR), one thing which didn't really change (one could generate tzinit & friends in a hacky way even before, too) and one thing about which I'm still not convinced that it improved something over the existing solution (the build folder).

Ideally, mintlib (and others) should drop this non-standard "holy trinity" and generate just files according to supplied CFLAGS for one target, e.g. something like make PREFIX=/m68k-atari-mintelf CFLAGS='-O -m68020-60 -g' LIBDIR=m68020-60 SUFFIX=_g. That would be something worth changing for sure.

@th-otto
Copy link
Contributor

th-otto commented May 21, 2024

  • libraries are now (partially) stripped while building

Hm, i have to check, but i can't remember to have changed that. Which libraries exactly are affected?

why is the former useful? And even used in the build scripts now?

I think i have already mentioned that: using prefix is plain wrong, because it makes it impossible to install to directories like /sbin and /etc

make install prefix=<prefix> output:

sbin/tzinit, tzselect, zdump, zic

And that is wrong. tzinit belongs to /sbin, not /usr/sbin

I suppose the idea is to have those three archives downloadable & installable for the freemint snapshots

Yes, exactly.

make & make install which just install the usual files without any executables (so the main archive wouldn't contain any either)

I left the (68000) binaries in the other archives, because the current build scripts expect them to find them there. Once the scripts have been adjusted, they can be removed from the archives.

build folder doesn't contain all generated files, not even all object files

Which ones do you think are missing?

sunrpc, syscall and tz folders, object files are still generated in the source directory for those folders what is really ironic >because it is -m68000 object files which are generated there.

Those directories are different, because they are also compiled with the host compiler. syscall for example is used to generate the tool that generates the trap*.h files, and tz is compiled twice, because we need a host tool that can generate the timezone database. Their object files therefor don't belong to the directories where the libraries are built.

i fail to see what is the advantage of the new structure then?

The old build system had a dozen or so directories in the toplevel, when you also create -mfastcall libraries. And all of them also had Makefiles in them, with their own directory name again hardcoded, so you cannot even remove them. Now you just do rm -rf build to clean up, which is sometimes needed if you need to rename/move source files, because the .deps files still reference the old names.

@mikrosk
Copy link
Member Author

mikrosk commented May 22, 2024

Hm, i have to check, but i can't remember to have changed that. Which libraries exactly are affected?

Every library -- not only libc.a but also the others, basically everything which has ranlib run on it. Before it was just ranlib, now it has a strip step, too.

And that is wrong. tzinit belongs to /sbin, not /usr/sbin

Ok, that might be true but your DESTDIR solution does the other bad thing, it forces usr on others. With all the initiatives to merge /bin and /usr/bin (etc) in modern distros, I don't consider the ability to use /sbin and /usr/sbin separately as crucial... if you really like the separation, please come up with a solution which doesn't force the user to use /usr.

I left the (68000) binaries in the other archives, because the current build scripts expect them to find them there. Once the scripts have been adjusted, they can be removed from the archives.

Which scripts do you mean? mintlib's?

Now you just do rm -rf build to clean up, which is sometimes needed if you need to rename/move source files, because the .deps files still reference the old names.

Yes, that's fine but if the goal is to have build with such nice features, it should contain all object files, i.e. also those tz and rpc stuff. Otherwise it is just more confusing (again, I understand why it ended up in state like this but if we are about to change something as major as this, it should be done properly for all object files).

@th-otto
Copy link
Contributor

th-otto commented May 22, 2024

please come up with a solution which doesn't force the user to use /usr.

But that is our current filesystem standard. The prefix is for example also hardcoded in paths like /usr/share/zoneinfo, /usr/share/terminfo and in various pathnames in include/paths.h

You can still call install the libraries with something like make install DESTDIR=/tmpdir PREFIX=/opt/somewhere, just the name of the toplevel-directory where to do the install should not be part of PREFIX

Which scripts do you mean? mintlib's?

No, those from freemint that create the cpu/bootable builds

it should contain all object files, i.e. also those tz and rpc stuff.

Yes, maybe. As already mentioned, that requires some cleanup of the checkrules which currently duplicates lots of things. But it is also easy to break things there ;)

@mikrosk
Copy link
Member Author

mikrosk commented May 22, 2024

But that is our current filesystem standard. The prefix is for example also hardcoded in paths like /usr/share/zoneinfo, /usr/share/terminfo and in various pathnames in include/paths.h

Hmm, I've checked the last 'official' mintlib https://web.archive.org/web/20040604211057/http://home.arcor.de/altf4/pkg/mintlib-0.56.1-1.m68kmint.tgz and it seems that you are right, it was like this before, too. On the other hand, for the past decade we have been supplying mintlib without the /usr folder and nobody complained.

No, those from freemint that create the cpu/bootable builds

But we don't package / use tzinit and others in those. We download mintlib only for building purposes for the freemint tools. So there's no need to keep the executables in mintlib archives even before fixing freemint/freemint#197.

that requires some cleanup of the checkrules which currently duplicates lots of things

I'm all for any cleanup in mintlib build scripts. The current state seems really over engineered.

@th-otto
Copy link
Contributor

th-otto commented May 22, 2024

But we don't package / use tzinit and others in those.

Yes, but we should, atleast in the bootable builds. Its part of the boot process after all, and normally also invoked by mint.cnf (unless you commented that out, haven't checked).

@mikrosk
Copy link
Member Author

mikrosk commented May 22, 2024

No arguments from me here but for that you don't need tzinit in the main archives, only in the -bin ones.

@mikrosk
Copy link
Member Author

mikrosk commented May 22, 2024

One more bug: #5 (comment) (make install-headers doesn't work).

@mikrosk
Copy link
Member Author

mikrosk commented May 23, 2024

Another bug: broken backward compatibility for make install prefix=<prefix>. Even if we agree to change the structure of distribution archive (i.e. /sbin and /usr in toplevel as mentioned in #68 (comment)), this is very dangerous: in this new version, it silently overwrites /usr/lib with every target, in our case it leaves ColdFire crt0.o, libc.a etc in there instead of 68000 stuff.

If others are like me, i.e. used just to type "make install PREFIX/prefix=<prefix> this will silently introduce fatal bugs, especially on a.out target (thank god for elf and coldfire, otherwise I wouldn't have noticed as well!)

@mikrosk
Copy link
Member Author

mikrosk commented May 23, 2024

Wait a second. Not only make install prefix=<prefix> doesn't work, make install DESTDIR=<prefix> doesn't either. What is quite puzzling because I see that

make SHELL=/bin/bash
make SHELL=/bin/bash DESTDIR="${INSTALL_DIR}" install
doesn't do anything special.

And yet, if I do

make DESTDIR=/home/mikro/gnu-tools/m68000/m68k-atari-mintelf/sys-root CROSS_TOOL=m68k-atari-mintelf SHELL=/bin/bash
make DESTDIR=/home/mikro/gnu-tools/m68000/m68k-atari-mintelf/sys-root CROSS_TOOL=m68k-atari-mintelf SHELL=/bin/bash install

before and after 05590c6, the former works while the latter doesn't (all libs are put into sys-root/lib without any cpu subfolders).

@th-otto can you try make install on latest master locally on your setup?

@th-otto
Copy link
Contributor

th-otto commented May 24, 2024

Yes, works here as expected:

../../mkinstalldirs /tmp/t/usr/lib/m68020-60
 install -m 644 libc.a /usr/lib/m68020-60
 install -m 644 libiio.a /usr/lib/m68020-60
 install -m 644 librpcsvc.a /usr/lib/m68020-60
 install -m 644 crt0.o /usr/lib/m68020-60
 install -m 644 gcrt0.o /usr/lib/m68020-60
...
../../mkinstalldirs /tmp/t/usr/lib/m5475
 install -m 644 libc.a /usr/lib/m5475
 install -m 644 libiio.a /usr/lib/m5475
 install -m 644 librpcsvc.a /usr/lib/m5475
 install -m 644 crt0.o /usr/lib/m5475
 install -m 644 gcrt0.o /usr/lib/m5475
...
etc.

You can also check the snapshot archives, which have all the cpu subfolders.

@mikrosk
Copy link
Member Author

mikrosk commented May 24, 2024

Yes, I saw the snapshots, that's why I asked for a local build, to rule out some hidden env variables on Github or something.

The only thing I can think of is that the issue is that I compile it with multilib-less gcc but I don't see anything in the offending commit which would rely on it?

@th-otto
Copy link
Contributor

th-otto commented May 24, 2024

Wtf? Why with multilib-less gcc? That won't work of course.

instdir=`$(CC) $$cflags -print-multi-directory`; \

@mikrosk
Copy link
Member Author

mikrosk commented May 24, 2024

Ah right, so that explains it then. As of why: #5 (comment), I use --disable-multilib in my preliminary gcc to speed up building.

Why was such a change needed at all? It worked fine before without asking CC for its multilib setup.

@th-otto
Copy link
Contributor

th-otto commented May 24, 2024

I use --disable-multilib in my preliminary gcc to speed up building.

But that's nonsense. It would use the wrong startup files/libraries.

Why was such a change needed at all? It worked fine before without asking CC for its multilib setup.

You get what you asked for. A single directory instead of a multilib setup.

The change was needed to get the fastcall versions installed in the correct place. Same would be the case if we ever re-introduce a -mshort version.

@mikrosk
Copy link
Member Author

mikrosk commented May 24, 2024

You get what you asked for. A single directory instead of a multilib setup.

Yes, built and copied three times to the same place. It wasn't happening before and it is now, so I hardly can call this an improvement.

Compiling mintlib's multilib targets with a multilib-less compiler is a valid expectation; strange that you argue about the value of keeping mshort, sozobon and pure c ("At least mshort and Pure-C were both supported at some time in the past, and i would call it a regression that they don't work anymore") but within a blink of eye you are totally OK to fatally break backward compatibility here.

@th-otto
Copy link
Contributor

th-otto commented May 24, 2024

Yes, built and copied three times to the same place. It wasn't happening before and it is now, so I hardly can call this an improvement.

It is an improvement. Previously, with such a gcc, you would the same library copied to three different places, which is simply wrong.

OK to fatally break backward compatibility here.

I'm not breaking anything here. The Makefiles for mintlib were always designed to compile all flavours of libraries. It's your broken build system that does not work.

@mikrosk
Copy link
Member Author

mikrosk commented May 24, 2024

It is an improvement. Previously, with such a gcc, you would the same library copied to three different places, which is simply wrong.

This is not true. I'd get nice three -m68000 / -m68020-60 / -mcpu=5475 builds in three different places. I'm not making things up, it I say it worked, it worked.

The Makefiles for mintlib were always designed to compile all flavours of libraries.

And? I want to do exactly that, build all three flavours and now I can't.

It's your broken build system that does not work.

I would like to remind you that it was me who first brought m68k gcc / binutils on github / travis and 10 years ago, using this very "broken build system". Now you come, change mintlib without improving anything substantial, break my scripts and tell me that it's my fault. Not very considerate.

@mikrosk
Copy link
Member Author

mikrosk commented Jun 6, 2024

I have forced myself to rework my build scripts so I have solved the issues with prefix/DESTDIR and multilib. So what I would like to see fixed in for this issue is:

  • the forced stripping
  • the forced compilation and installation of m68k tzinit & friends when cross compiling and make install (I can live with the hacky way to compile all three CPU flavors unless tzinit (and others) should build also for -m68020/-mcpu=5475 #60 is fixed but it's annoying to have m68000 build&installed no matter what)

@th-otto
Copy link
Contributor

th-otto commented Jun 7, 2024

the forced stripping

I still don't see why this is a problem. For a.out, there is no point in providing a library with debug symbols, and for elf there are the libc_g.a libraries.

the forced compilation and installation of m68k tzinit & friends

I thought they would be needed by the scripts in freemint, but apparently currently aren't. So they can be removed from library only archives.

@mikrosk
Copy link
Member Author

mikrosk commented Jun 7, 2024

I still don't see why this is a problem. For a.out, there is no point in providing a library with debug symbols, and for elf there are the libc_g.a libraries.

But it's not up to you, me or anyone else to decide whether and why it is important. We provide semi-official snapshots for everyone so we should make them as versatile as possible.

I thought they would be needed by the scripts in freemint, but apparently currently aren't. So they can be removed from library only archives.

Sure, that's what I do in my gcc scripts as well but the proper fix is just ... not do it.

@th-otto
Copy link
Contributor

th-otto commented Jun 7, 2024

But we have always stripped the libraries, if not in the makefile, then after installing them.

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

No branches or pull requests

2 participants