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

v17.0 Build Failure #79

Closed
briansalehi opened this issue Oct 1, 2024 · 14 comments
Closed

v17.0 Build Failure #79

briansalehi opened this issue Oct 1, 2024 · 14 comments

Comments

@briansalehi
Copy link
Contributor

Hey everyone, postgres 17.0 is out, and as expected, everybody would run that sweet command pgenv build 17.0 to enjoy the new features. Well, the build failed for me for some reason, so I ran the pgenv executable with bash -x bin/pgenv to see what went wrong (is there any other way to run with verbose output?).

It failed in make step because of parsing documentation files. Apparently the world target would also build docs, so I tried to run make all instead, then the build succeeded.

Could you try v17.0 and see if pgenv works for you? If not, what's the new dependency making builds fail?

@briansalehi briansalehi changed the title v17.0 Build v17.0 Build Failure Oct 1, 2024
@briansalehi
Copy link
Contributor Author

From ages ago, this dependency made the build fail occasionally (https://postgrespro.com/list/thread-id/1781188)

So the dependency was docbook-xsl package on Debian/Ubuntu, docbook-style-xsl on Fedora. Now world target also works for make. I'm not sure if pgenv should be modified to cover that or not! pgenv only checks for dependencies required for its own to run, but is it a good idea to also check if dependencies for the postgres also exist? Actually configure script is responsible for that but for some reason it didn't warn me about the missing docbook-xsl!

@fluca1978
Copy link
Collaborator

I confirm everything is working :

% pgenv build 17.0


 % pgenv switch 17.0

% psql -h localhost -U postgres
psql (16.4 (Ubuntu 16.4-0ubuntu0.24.04.1), server 17.0)
WARNING: psql major version 16, server major version 17.
         Some psql features might not work.
Type "help" for help.

postgres=# select version();
                                             version                                              
--------------------------------------------------------------------------------------------------
 PostgreSQL 17.0 on x86_64-pc-linux-gnu, compiled by gcc (Ubuntu 13.2.0-23ubuntu4) 13.2.0, 64-bit
(1 row)

therefore I close the issue.

With regard to the dependencies, I don't think it is a good idea to make pgenv aware of all dependencies on every PostgreSQL version, the output of make world should help the user to get an idea of what is missing on her machine.
I think listing the PostgreSQL's dependencies is too much work for this kind of application.

@theory
Copy link
Owner

theory commented Oct 3, 2024

Does make world succeed if docbook-xsl isn't installed?

@theory theory reopened this Oct 3, 2024
@briansalehi
Copy link
Contributor Author

@theory unfortunately not. At first, I thought this is just a dependency missing on my laptop at work (Ubuntu 24.04), but when I tried it at home (Fedora 14) it failed with same exact error. In both cases docbook-xsl is required. Interestingly, other previous releases work just fine (16.x).
So this is out of the scope of pgenv anyways, but at first sight I was wondering why errors never happened for me so far, so I suspected it might be pgenv. When I checked, I realized it's a dependency. But yet, I'm still wondering what's the difference between the world target and all? Can we change pgenv to try another target as a back up when one fails?

@theory
Copy link
Owner

theory commented Oct 3, 2024

@briansalehi I assume you're not building from a Git clone, yes?

@briansalehi
Copy link
Contributor Author

@theory you mean the pgenv? I do, I cloned this repository a long time ago when I first started reading the book. So I pull changes when there's any, and I made the bin directory in this repo an entry to my PATH. And simply run pgenv build 17.0. Is there any other way around I didn't pay attention to?

@theory
Copy link
Owner

theory commented Oct 3, 2024

No, I meant using PGENV_LOCAL_POSTGRESQL_REPO to build Postgres from a clone of its repository. That would require docbook-xml. But the release should not, because the XML docs are built into it. Or they used to be. Hmmm…

@theory
Copy link
Owner

theory commented Oct 3, 2024

Okay, turns out that this change removed the pre-compiled files, including docs. So now we have to decide what to do:

  1. Require the xml-docbook package for 17+ and keep using make world
  2. Use the new make world-bin target on 17+ and just document somewhere that 17 no longer includes docs
  3. Somehow detect ahead of time whether xml-docbook is available and build docs if possible. Maybe there's a separate make docs target of some kind we can use and trap and ignore errors?
  4. There appears to be a separate docs tarball now, e.g., postgresql-17.0-docs.tar.gz; maybe also download it and avoid the issue?

@briansalehi
Copy link
Contributor Author

@theory I would definitely not try options 1 and 3 because it would require pgenv to look into user packages, considering many different distributions and different package names. This would result in endless headaches, even though it seems the most intuitive solution to this problem.

didn't know we have world-bin target which basically is world without docs.
So, options 2 and 4 in combination seem the best idea to me.
We could build without docs and then install docs package separately!

@briansalehi
Copy link
Contributor Author

briansalehi commented Oct 3, 2024

@theory You're right, there is a docs target. We can run it and let the user know about the dependency when docs failed. This would lead to the least changes to pgenv, while also supporting 17+ version!
The harder way would be to check for docs target first and if it failed then we try world-bin and external docs tarball.

@fluca1978
Copy link
Collaborator

4. There appears to be a separate `docs` tarball now, e.g., [postgresql-17.0-docs.tar.gz](https://ftp.postgresql.org/pub/source/v17.0/postgresql-17.0-docs.tar.gz); maybe also download it and avoid the issue?

Seems to me the more sane option so far, I don't think pgenv should know about dependencies to buil PostgreSQL because it will lead to a mess of "you need just another one package" hell.

@briansalehi
Copy link
Contributor Author

I would say we replace world target with world-bin, and install docs from tarball without checking for versions!
The problem with docs was known from earlier versions. This could be a failure point in build that we missed the whole time. It will not be a breaking change and users will not notice anything. What do you think?

@fluca1978
Copy link
Collaborator

I would say we replace world target with world-bin, and install docs from tarball without checking for versions!

Agree if all versions have a doc tarball. Or even skip the tarball about docs at all, and provide a build-all command to handle the building of (also) docs.

@briansalehi
Copy link
Contributor Author

I just checked and only the +17 have docs tarballs because of the recent change in docs generation and removal of prebuilt binaries. Well, then this problem only happens from now on, then we have to check for version anyways.

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

3 participants