shadow-maint / shadow

Upstream shadow tree
Other
290 stars 228 forks source link

fix make distcheck #970

Closed hallyn closed 4 months ago

hallyn commented 4 months ago

This is to fix issue #948.

Almost there. For some reason, the man/po/shadow-man-pages.pot file still gets updated when only the pot creation date is changed... it seems like the Makefile should be preventing that. Will look at that tomorrow.

hallyn commented 4 months ago

At this point, there is a remaining distcheck failure because it wants to rebuild the manpages, but doesn't have the config.xml to include in the other *.xmls. Really, the answer is probably that it should not regenerate the docs.

hallyn commented 4 months ago

Passes make distcheck now. Needs some rebasing though.

hallyn commented 4 months ago

(Rebasing done, pr should be ready)

alejandro-colomar commented 4 months ago

BTW, another thing that bugs me is (not of this patch; of the build system in general), after a build:

$ git status
On branch 2024-03-13/fix-distcheck
Your branch is up to date with 'sh/2024-03-13/fix-distcheck'.

Untracked files:
  (use "git add <file>..." to include in what will be committed)
        lib/atoi/.dirstamp
        lib/string/.dirstamp
        man/da/login.defs.d
        man/da/messages.mo
        man/de/login.defs.d
        man/de/messages.mo
        man/fr/login.defs.d
        man/fr/messages.mo
        man/it/login.defs.d
        man/it/messages.mo
        man/pl/login.defs.d
        man/pl/messages.mo
        man/ru/login.defs.d
        man/ru/messages.mo
        man/sv/login.defs.d
        man/sv/messages.mo
        man/uk/login.defs.d
        man/uk/messages.mo
        man/zh_CN/login.defs.d
        man/zh_CN/messages.mo

nothing added to commit but untracked files present (use "git add" to track)
eli-schwartz commented 4 months ago

BTW, another thing that bugs me is (not of this patch; of the build system in general), after a build:

It's not the build system's fault. It's the gitignore's fault: https://github.com/shadow-maint/shadow/pull/972

alejandro-colomar commented 4 months ago

BTW, another thing that bugs me is (not of this patch; of the build system in general), after a build:

It's not the build system's fault. It's the gitignore's fault: #972

Yeah, it's another way of looking at it, I guess. But the build system could be nicer and not pollute the repository so much. I'm thinking of: https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/tree/.gitignore and https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/tree/share/mk/clean.mk#n14.

Yeah, I know I could do out-of-tree builds, but I still saw some changes in the repo (and it's a bit more uncomfortable). I see Serge's patch fixed some of that (I don't remember if there's anything remaining).

Anyway, thanks for the patch!

eli-schwartz commented 4 months ago

Yeah, it's another way of looking at it, I guess. But the build system could be nicer and not pollute the repository so much. I'm thinking of: https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/tree/.gitignore and https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/tree/share/mk/clean.mk#n14.

This is just out of tree builds, anyway...

alejandro-colomar commented 4 months ago

Yeah, it's another way of looking at it, I guess. But the build system could be nicer and not pollute the repository so much. I'm thinking of: https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/tree/.gitignore and https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/tree/share/mk/clean.mk#n14.

This is just out of tree builds, anyway...

The problem is that out-of-tree builds in this project still produce some changes in the tree:

alx@debian:~/src/shadow/shadow/master$ git status
On branch master
Your branch is up to date with 'gh/master'.

nothing to commit, working tree clean
alx@debian:~/src/shadow/shadow/master$ git clean -dffx
alx@debian:~/src/shadow/shadow/master$ mkdir ../tmp
alx@debian:~/src/shadow/shadow/master$ cd ../tmp/
alx@debian:~/src/shadow/shadow/tmp$ ../master/autogen.sh |& wc -l
348
alx@debian:~/src/shadow/shadow/tmp$ make -j24 |& wc -l
8110
alx@debian:~/src/shadow/shadow/tmp$ echo $?
0
alx@debian:~/src/shadow/shadow/tmp$ cd ../master/
alx@debian:~/src/shadow/shadow/master$ git status
On branch master
Your branch is up to date with 'gh/master'.

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
        modified:   po/fr.po
        modified:   po/ka.po

Untracked files:
  (use "git add <file>..." to include in what will be committed)
        test-driver

no changes added to commit (use "git add" and/or "git commit -a")

This shouldn't happen.

alejandro-colomar commented 4 months ago

Ah, and I see there are even more changes to the tree:

$ git clean -dffx
Removing ABOUT-NLS
Removing Makefile.in
Removing aclocal.m4
Removing autom4te.cache/
Removing compile
Removing config.guess
Removing config.h.in
Removing config.rpath
Removing config.sub
Removing configure
Removing contrib/Makefile.in
Removing depcomp
Removing doc/Makefile.in
Removing etc/Makefile.in
Removing etc/pam.d/Makefile.in
Removing etc/shadow-maint/Makefile.in
Removing install-sh
Removing lib/Makefile.in
Removing libsubid/Makefile.in
Removing ltmain.sh
Removing m4/
Removing man/Makefile.in
Removing man/cs/Makefile.in
Removing man/da/Makefile.in
Removing man/de/Makefile.in
Removing man/es/Makefile.in
Removing man/fi/Makefile.in
Removing man/fr/Makefile.in
Removing man/hu/Makefile.in
Removing man/id/Makefile.in
Removing man/it/Makefile.in
Removing man/ja/Makefile.in
Removing man/ko/Makefile.in
Removing man/pl/Makefile.in
Removing man/pt_BR/Makefile.in
Removing man/ru/Makefile.in
Removing man/sv/Makefile.in
Removing man/tr/Makefile.in
Removing man/uk/Makefile.in
Removing man/zh_CN/Makefile.in
Removing man/zh_TW/Makefile.in
Removing missing
Removing po/Makefile.in.in
Removing po/Makevars.template
Removing po/Rules-quot
Removing po/boldquot.sed
Removing po/bs.gmo
Removing po/ca.gmo
Removing po/cs.gmo
Removing po/da.gmo
Removing po/de.gmo
Removing po/dz.gmo
Removing po/el.gmo
Removing po/en@boldquot.header
Removing po/en@quot.header
Removing po/es.gmo
Removing po/eu.gmo
Removing po/fi.gmo
Removing po/fr.gmo
Removing po/fr.po~
Removing po/gl.gmo
Removing po/he.gmo
Removing po/hu.gmo
Removing po/id.gmo
Removing po/insert-header.sin
Removing po/it.gmo
Removing po/ja.gmo
Removing po/ka.gmo
Removing po/ka.po~
Removing po/kk.gmo
Removing po/km.gmo
Removing po/ko.gmo
Removing po/nb.gmo
Removing po/ne.gmo
Removing po/nl.gmo
Removing po/nn.gmo
Removing po/pl.gmo
Removing po/pt.gmo
Removing po/pt_BR.gmo
Removing po/quot.sed
Removing po/remove-potcdate.sin
Removing po/ro.gmo
Removing po/ru.gmo
Removing po/sk.gmo
Removing po/sq.gmo
Removing po/sv.gmo
Removing po/tl.gmo
Removing po/tr.gmo
Removing po/uk.gmo
Removing po/vi.gmo
Removing po/zh_CN.gmo
Removing po/zh_TW.gmo
Removing src/Makefile.in
Removing test-driver
Removing tests/unit/Makefile.in
Removing ylwrap

These are probably gitignored, because they didn't appear in the git changes.

I would argue that gitignoring /test-driver is just hiding the problem, not fixing it.

eli-schwartz commented 4 months ago

I think that it's not evident everyone is on the same page about what constitutes "the problem"...

alejandro-colomar commented 4 months ago

Out-of-tree builds in the Linux man-pages repo are actually out-of-tree. And they are the only kind of builds that can happen (unless you somehow specify that builddir=., in which case I don't know what'll happen; just don't do that). So the tree always remains clean. That's what I think should be the sane behavior of a build system, and is one of the many reasons autotools makes me not-so-happy.

alejandro-colomar commented 4 months ago

I think that it's not evident everyone is on the same page about what constitutes "the problem"...

"The problem" is actually two problems.

The main one, is blindly running git add . after writing some changes and building+testing them, in which case you could slip in some unwanted file. That's solved with the gitignore.

The second one, is doing other stuff with git, such as git clean -dffx, and other things that affect those gitignored files, which breaks out-of-tree builds:

alx@debian:~/src/shadow/shadow/master$ git clean -dffx >/dev/null
alx@debian:~/src/shadow/shadow/master$ cd ../tmp/
alx@debian:~/src/shadow/shadow/tmp$ make
CDPATH="${ZSH_VERSION+.}:" && cd ../master && /bin/bash '/home/alx/src/shadow/shadow/master/missing' aclocal-1.16 
/bin/bash: /home/alx/src/shadow/shadow/master/missing: No such file or directory
make: *** [Makefile:431: ../master/aclocal.m4] Error 127

There might be other subproblems, but those are the main ones I remember at the moment.

eli-schwartz commented 4 months ago

Out-of-tree builds in the Linux man-pages repo are actually out-of-tree. And they are the only kind of builds that can happen (unless you somehow specify that builddir=., in which case I don't know what'll happen; just don't do that). So the tree always remains clean. That's what I think should be the sane behavior of a build system, and is one of the many reasons autotools makes me not-so-happy.

It very much misses the point that autotools is fully out of tree, and it is also a tool for generating a standalone portable build system with actually-correct support for building libraries and handling configure-time options that isn't checked into git, but does need to live in the source tree per definition.

eli-schwartz commented 4 months ago

The second one, is doing other stuff with git, such as git clean -dffx, and other things that affect those gitignored files, which breaks out-of-tree builds:

It doesn't break out of tree builds. It just deletes the autotools-generated build system, so if you do it you'll have to regenerate the autotools.

There's no real reason to do git clean -dffx because, after all, you are using an out of tree build system -- so just delete that.

Or just git clean without -x. Or just use make clean or possibly even make distclean.

I'm genuinely unsure why you feel it is so important to git clean -dffx, as it's not something I'm particularly accustomed to needing with ANY build system, autotools or otherwise.

alejandro-colomar commented 4 months ago

The second one, is doing other stuff with git, such as git clean -dffx, and other things that affect those gitignored files, which breaks out-of-tree builds:

It doesn't break out of tree builds. It just deletes the autotools-generated build system, so if you do it you'll have to regenerate the autotools.

There's no real reason to do git clean -dffx because, after all, you are using an out of tree build system -- so just delete that.

Or just git clean without -x. Or just use make clean or possibly even make distclean.

I'm genuinely unsure why you feel it is so important to git clean -dffx, as it's not something I'm particularly accustomed to needing with ANY build system, autotools or otherwise.

I do it because it's portable. When I start working on some random project, whose build system I don't know, I never know if make clean will be enough, or if it will be available at all. git restore .; git clean -dffx is a portable way of saying "I want a clean git repository".

Because I don't need in-repo temporary files in my projects, it's also a way of cleaning up temporary files that an editor might have left over and I didn't notice (e.g., I forgot to close vim(1) when I turned off my computer), knowing that I won't break anything, since there shouldn't be anything. It also helps if I want to grep for something, and don't want to see generated files in my output.