Shouldn’t this sort of thing be caught by continuous integration jobs, though?

It appears that it was removed by #1001 due to being "broken". I definitely think that we should fix and restore it.

It appears that it was removed by #1001 due to being "broken". I definitely think that we should fix and restore it.

I think we need something like this: https://github.com/zonemaster/zonemaster-engine/blob/master/t/manifest.t

@tgreenx and @marc-vanderwal: Today we update MANIFEST and MANIFEST.SKIP manually. Instead we could run make manifest and having MANIFEST being automatically updated with anything not matching MANIFEST.SKIP. That could be included in make all.

There are two possible strategies for handling the MANIFEST file.

Either we keep it under version control as we currently do. This comes with the implicit assumption that the MANIFEST be constantly in sync with the rest of the files under version control. That means it must be updated manually, and it is an error to commit a change that adds or deletes a file under version control and fails to update the MANIFEST. An automated test checking the package contents against the MANIFEST is therefore very desirable.

ExtUtils::MakeMaker provides make distcheck, that will do just that. It doesn’t set the exit status to 1 if there is a discrepancy, however, so it isn’t suitable for running in a CI pipeline; but a similar result can be achieved by hooking into ExtUtils::Manifest::fullcheck() directly from a Perl one-liner, like so:

# With the current working directory being the repository root
perl -MExtUtils::Manifest=fullcheck -e '($m,$e)=fullcheck; exit !(!@$m & !@$e)'

The alternative is not to keep the MANIFEST under version control and only keep MANIFEST.SKIP. Only then does it make sense to have the MANIFEST generated automatically during CI jobs or preparation of packages. Then it isn’t necessary to check the installed files against MANIFEST anymore, because then, the assumption is that MANIFEST.SKIP is always right. But I don’t really like this route, because then, there is no way of ensuring that the files being installed by the package are exactly what is necessary, no more and no less, at each PR.

I’m perfectly fine with the first solution, provided that we restore some kind of check to ensure that the MANIFEST stays in sync.

(PS: Yes, I have to admit that making the shortest one-liner that works as intended is a fun exercise.)

@marc-vanderwal, in Zonemaster-Engine we have a working solution for checking if MANIFEST has been updated. I uses make distcheck and checks if the output is empty or non-empty. See https://github.com/zonemaster/zonemaster-engine/blob/master/t/manifest.t. It works under CI (Githbu actions).

In this repository we have no such check, and as far as I remember we have never had such check. I tried in #1137, but I could not make it work. I could not see that the MO files are generated, and then make distcheck will fail. Here we use Travis for CI.

The value of manually updating MANIFEST so I suggest that we switch to running make manifest before make dist. Of course then we do not need to keep MANIFEST under version control. We should still update MANIFEST.SKIP as needed.