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

Preparation for release 2.4.10 #245

Closed
mjeveritt opened this issue Jan 15, 2021 · 47 comments
Closed

Preparation for release 2.4.10 #245

mjeveritt opened this issue Jan 15, 2021 · 47 comments

Comments

@mjeveritt
Copy link
Contributor

Per #111 (comment) , let's start discussion on any issues arising from release 2.4.9 and patches we'd like to integrate into the Next release (expected 2.4.10).

I shall be PR'ing patches shortly from Gentoo per @Polynomial-C 's request.

@kloczek
Copy link

kloczek commented Feb 6, 2021

Just a suggestion to make releases a bit more frequently :)
As still flow of the patches is relatively high it would better to allow ebvaluate accepted commits in smaller chunks to allow easier identify some changes. I think that new release after ot more than 50 commits would be kind of optimum for ppp.

@EasyNetDev
Copy link
Contributor

EasyNetDev commented Jun 17, 2021

I agree. Even the replies / pull requests are very slow. This tool is very useful almost in any route deploying if you need to build a VPN / AC router.
I found recently issues in slprintf and strlcpy functions.
I know that are some improved functions of snprinf and strncpy, but I think they are outdated.
slprinf is not processing correctly the formats. Instead replacing the arguments is displaying the format instead, example:

//  slprintf(buf, sizeof(buf), "%08lX%04X%02hX",
  snprintf(buf, sizeof(buf), "%08lX%04X%02hX",
           (unsigned long int) time (NULL),
           (unsigned int) getpid (),
           cnt & 0xFF);

slprintf is putting in buf value "%lX60cb2c01%hX". snprintf on the other hand is putting in buf this value: "60CB2E2F2459F900".

I don't know the reasons why these functions where replicated, but I think we can rewrite them using standard C lib funtions and to behave same as actual sl.. functions.

Opinions?

@ghost
Copy link

ghost commented Jul 9, 2021

Hi there, I'm from Gentoo musl. Please take a look on #293 too, more info inside pr

@paulusmack
Copy link
Collaborator

As to why we have slprintf still, the main reason is that it has some useful formats that the standard C *printf functions don't have, for example, %I (IP address), %q (quoted string), %v ("visible" string, i.e. non-printing chars translated to a printing form), %t (time and date), %B (print buffer in hex), %P (print PPP packet).

You're right it didn't have %X, but that is fixed now.

@paulusmack
Copy link
Collaborator

I think the next release should be called 2.5.0, given the major change to the build system. I would like to do it in early January 2022.

@Neustradamus
Copy link
Member

@paulusmack: Thanks for last merging :)

There was a problem with PR of @EasyNetDev, maybe good to look commits?

Have you seen @enaess comments here:

About OpenSSL 3.0.0: Have you remarks?

@enaess
Copy link
Contributor

enaess commented Aug 16, 2022

@Neustradamus There is a bunch of issues that either can be closed or triaged. You able to go through the set of issues and labeling them a:bug or a:feature-request or a:must-fix, or similar schema?

@Neustradamus
Copy link
Member

Neustradamus commented Dec 27, 2022

I think last PRs before the new build will be:

@Neustradamus
Copy link
Member

About a 2.5.0 build:

Hope it will be good!

@rfc1036
Copy link
Contributor

rfc1036 commented Jan 7, 2023

I have prepared a tentative updated Debian package with a current snapshot: https://salsa.debian.org/debian/ppp/-/commits/master-md , while also removing most patches. No significant issues, and the new autoconf-based build infrastructure is working very well even if I had to use different hacks: we could use, at least, switches for disabling crypt(3) and selecting which plugins should be built.

We will not be able to target the next Debian release (12) because a library transitions freeze is scheduled to happen in one week, but I may make backports later if needed. Hopefully an official public API will make this much easier in the future.

@enaess
Copy link
Contributor

enaess commented Jan 7, 2023

Thanks for all that work @rfc1036!

I think @bootc similarly concluded the timeframe to get pppd 2.5.0 into bookworm (12) release would be near impossible in an email to @Neustradamus and @paulusmack and myself. Packaging it for a Bookworm release should perhaps started 6 months ago together with the proposed header-file re-organization, and deal with the breakage in existing packages.

Regardless, I think 2.5.0 with the latest set of patches would be helpful when going forward; so we should proceed with a release despite it being too late for Debian. If we leave a bit of room to take your suggestions e.g. stuff you need to patch perhaps should go upstream and directly to pppd? Also, with pkg-config, does the need for dh_ppp script go away entirely as sister packages should just use that instead of trying to infer the version in any other way? Any other variables you think would be helpful to the pkg-config here?

Did you also get to produce a stripped down ppp udev variant? Or do you need maybe another configure option to leave out crypt.h and friends? Any other features you see lacking from ./configure?

@rfc1036
Copy link
Contributor

rfc1036 commented Jan 7, 2023

We could probably have done it with a couple more weeks, but it is how it is.

I am waiting for @bootc's review to upload that snapshot to Debian/experimental: there is no need to wait for a release.

After that (or even right now, if it were already merged), I would be happy to get a new snapshot with the reorganized headers setup, to really test how that works. I am not sure how much of dh_ppp and the symbols file machinery will still be needed, since I have not looked at the patch. The best case scenario would be to have the ppp package provide a ppp-api-TIMESTAMP virtual package that plugin packages could depend on, but I think that we will still need dh_ppp to automatically provide the right name. pkgconfig does not help distributions because we already know where the headers are (actually: in the default path) but we need something special to manage the API version since this is not about linking a program with a library.

For the udeb package I am disabling crypt(3), /etc/shadow support and some plugins by editing config.h and the generated makefiles, and for the time being that's enough. Just adding configure options to disable crypt(3) to and list the desired plugins should be trivial, but I am not very good at autoconf.

I would appreciate libtool not installing the useless .a and .la files, but we can still delete them later.

@bootc
Copy link
Contributor

bootc commented Jan 8, 2023

I am waiting for @bootc's review to upload that snapshot to Debian/experimental: there is no need to wait for a release.

Thanks for all the work on that @rfc1036; there are a couple of tweaks I'd like to do before uploading, mostly related to the symbol versioning machinery, and we should probably use a 2.5.0~ revision rather than 2.4.10~ given the discussion in this thread. I'll sort that out.

After that (or even right now, if it were already merged), I would be happy to get a new snapshot with the reorganized headers setup, to really test how that works. I am not sure how much of dh_ppp and the symbols file machinery will still be needed, since I have not looked at the patch. The best case scenario would be to have the ppp package provide a ppp-api-TIMESTAMP virtual package that plugin packages could depend on, but I think that we will still need dh_ppp to automatically provide the right name. pkgconfig does not help distributions because we already know where the headers are (actually: in the default path) but we need something special to manage the API version since this is not about linking a program with a library.

The reason we don't use a virtual package is that there are packages that don't strictly depend on ppp, just list it inRecommends even though they ship a plugin file. We don't want to make ppp a hard dependency in this case but they still need a way of saying they are only compatible with a certain range of versions of ppp. Hence the mechanism I came up with. The pkg-config script may significantly simplify dh_ppp but it won't remove the need for it entirely.

For the udeb package I am disabling crypt(3), /etc/shadow support and some plugins by editing config.h and the generated makefiles, and for the time being that's enough. Just adding configure options to disable crypt(3) to and list the desired plugins should be trivial, but I am not very good at autoconf.

Does the installer's libc still not have crypt() support? I'll ask the installer team and double-check.

I would appreciate libtool not installing the useless .a and .la files, but we can still delete them later.

That's easily fixed either on our end or upstream; I'll propose a PR for it.

@bootc
Copy link
Contributor

bootc commented Jan 8, 2023

Does the installer's libc still not have crypt() support? I'll ask the installer team and double-check.

These days we have the separate libxcrypt for crypt(3) and this has a libcrypt1-udeb package. I don't see why we should go out of our way to disable this support these days. @rfc1036?

@rfc1036
Copy link
Contributor

rfc1036 commented Jan 8, 2023

Even if I am the libxcrypt maintainer I am not sure of how many udeb packages actually use it. I suspect that at this point it is always pulled in, because it is needed to hash the passwords with the modern algorithm.

@enaess
Copy link
Contributor

enaess commented Jan 8, 2023

Cool!

Sounds like libxcrypt may solve that problem for us without having to do anything. Is there anything you found in packaging pppd, @rfc1036, that could be helpful for the project.

  • switch from libdir to datastoredir for installing (pkgconfig)
  • pathnames, connect-errors becomes ppp-connect-errors
  • others?

@rfc1036
Copy link
Contributor

rfc1036 commented Jan 8, 2023

Right now we have to maintain compatibility with the existing paths which have always been used by Debian, but if we could move/rename these files then they should be moved to /run/pppd/ and /var/log/pppd/. Except that Red Hat already uses /run/ppp/ and /var/log/ppp/ so we should follow them even if it is slightly suboptimal. The same applies to the TDB file.

@enaess
Copy link
Contributor

enaess commented Jan 8, 2023

@rfc1036 for /run/pppd and /var/log/pppd, isn't the --with-runtime-dir= and --with-logfile-dir= sufficient overrides for configure in this case?

@enaess
Copy link
Contributor

enaess commented Jan 8, 2023

@rfc1036

I would appreciate libtool not installing the useless .a and .la files

Is there a good way around it? if so I haven't yet found it. For the debian packages I maintain, I still had to modify my pkg recipe to nuke the files post "make install". Please enlighten me if you know how ..

@rfc1036
Copy link
Contributor

rfc1036 commented Jan 8, 2023

Of course, my point was that I do not recommend changing the defaults to the Debian paths. If they can be changed then at this point I suggest using the Red Hat ones.

I am not a libtool expert, but I understand that it is possible to stop building the static plugins with AC_ENABLE_STATIC(no).
There is no way to persuade libtool to not install the .la files.

@enaess
Copy link
Contributor

enaess commented Jan 10, 2023

@rfc1036 I tested the AC_ENABLE_STATIC(no) option, sure that doesn't produce the .a files. I think it is the equivalent of specifying --disable-static

Still, the .la files are being produced. They seem to be associated with the .so files installs (details about shared objects inside them).

@rfc1036
Copy link
Contributor

rfc1036 commented Jan 10, 2023

Yes, nothing can be done about the .la files, so distributions have to remove them manually. I will not get into the details, but the libtool maintainers feel really passionate about this...

@enaess
Copy link
Contributor

enaess commented Jan 10, 2023

Fair. I had to deal with this for my own Debian packages. I think it is a known / documented step you have to do to work around this.

@mjeveritt
Copy link
Contributor Author

Yes, nothing can be done about the .la files, so distributions have to remove them manually. I will not get into the details, but the libtool maintainers feel really passionate about this...

I can vouch that Gentoo Linux has a function in the build/deployment system that strips them out before installing.

@Polynomial-C
Copy link
Contributor

Yes, nothing can be done about the .la files, so distributions have to remove them manually. I will not get into the details, but the libtool maintainers feel really passionate about this...

I can vouch that Gentoo Linux has a function in the build/deployment system that strips them out before installing.

I's merely a find ... -type f -name '*.la' -delete call (which has to be placed in each and every ebuild) and not a specific function.

@mjeveritt
Copy link
Contributor Author

Yes, nothing can be done about the .la files, so distributions have to remove them manually. I will not get into the details, but the libtool maintainers feel really passionate about this...

I can vouch that Gentoo Linux has a function in the build/deployment system that strips them out before installing.

I's merely a find ... -type f -name '*.la' -delete call (which has to be placed in each and every ebuild) and not a specific function.

Derp my bad, we lost the built-in when that eclass got killed off ... 🙄

Also, Happy New Year Lars 😛

@Neustradamus
Copy link
Member

Some informations here:

@enaess
Copy link
Contributor

enaess commented Jan 11, 2023

Thanks @Neustradamus !

I believe Debian using similar construct in their rules/control files:

find ... -type f -name '*.la' -delete

Despite the annoyance, pppd doesn't use lt_load() function so this workaround is safe.

@yarda
Copy link
Contributor

yarda commented Jan 11, 2023

Hi, I tried Fedora scratch build from the HEAD and LGTM:
https://koji.fedoraproject.org/koji/taskinfo?taskID=96019866

One minor thing: you should probably add the copy of the licenses. E.g. AFAIK GPL requires the copy of the license to be included in the package/release.

@enaess
Copy link
Contributor

enaess commented Jan 11, 2023

@yarda - Could you please spell out what you mean by this, e.g. include a LICENSE.BSD-3WAY file or similar.

Also, given the different licenses floating around in the source code for pppd, it could serve as a good reason to break some of these "sub-modules" into separate projects under the ppp-project umbrella. For example, I'd love to see the pppoe plugin, pppoatm, and radius plugins be relocated in a different git repository with respective configure, CI builds, and correct licenses spelled out, and it could also make pppd easier to manage.

@enaess
Copy link
Contributor

enaess commented Jan 11, 2023

@rfc1036 @yarda one thing that is now possible is to add libatm-dev(el) to your build, and pppd would pick up the latet ATM library and link with it instead of it's private implementation of it. Just wanted to put that out there for package maintainers.

@yarda
Copy link
Contributor

yarda commented Jan 11, 2023

@yarda - Could you please spell out what you mean by this, e.g. include a LICENSE.BSD-3WAY file or similar.

Yes, and please also for the GPL, because there are some GPL licensed bits:
https://docs.fedoraproject.org/en-US/packaging-guidelines/LicensingGuidelines/#_license_text

Also, given the different licenses floating around in the source code for pppd, it could serve as a good reason to break some of these "sub-modules" into separate projects under the ppp-project umbrella. For example, I'd love to see the pppoe plugin, pppoatm, and radius plugins be relocated in a different git repository with respective configure, CI builds, and correct licenses spelled out, and it could also make pppd easier to manage.

This would be great. If there will be one tarball per new release, in Fedora we can still build multiple RPM sub packages from it. In case there will be multiple tarballs we can package them as multiple packages.

@yarda
Copy link
Contributor

yarda commented Jan 11, 2023

@rfc1036 @yarda one thing that is now possible is to add libatm-dev(el) to your build, and pppd would pick up the latet ATM library and link with it instead of it's private implementation of it. Just wanted to put that out there for package maintainers.

Added, in Fedora it's linux-atm-libs-devel package.

@enaess
Copy link
Contributor

enaess commented Jan 11, 2023

This would be great. If there will be one tarball per new release, in Fedora we can still build multiple RPM sub packages from it. In case there will be multiple tarballs we can package them as multiple packages.

I was thinking more like each of these plugins would live in it's own repository off the ppp-project, e.g. github.com/ppp-project/ppp-pppoe-plugin, or github.com/ppp-atm-plugin, each with a standalone .tar.gz and version, license, and project independence.

For your license requirements, it only seems like you need to know what to put in the "License:" field, no? I've seen other projects include a SPDX-License tag for each file (e.g. systemd). I am still a little unclear exactly code change you are proposing we are making for this.

@yarda
Copy link
Contributor

yarda commented Jan 11, 2023

I was thinking more like each of these plugins would live in it's own repository off the ppp-project, e.g. github.com/ppp-project/ppp-pppoe-plugin, or github.com/ppp-atm-plugin, each with a standalone .tar.gz and version, license, and project independence.

NP, I am OK with multiple packages. Each package will have to undergo Fedora review, but it's manageable.

For your license requirements, it only seems like you need to know what to put in the "License:" field, no? I've seen other projects include a SPDX-License tag for each file (e.g. systemd). I am still a little unclear exactly code change you are proposing we are making for this.

Yes, new packages have to add SPDX-License tag for every license found in the sources no matter what's the effective / resulting license of the package. But my previous request is about including file with the license text in the tarball package (see the linked fedora packaging doc for details). E.g. for the GPLv2: https://www.gnu.org/licenses/old-licenses/gpl-2.0.txt, just add the file gpl-2.0.txt to the package or LICENSE.GPL-2.0 (it's required by the GPL license) and you can also add LICENSE.BSD-3WAY (this is not required by the BSD license, but nice to have).

@enaess
Copy link
Contributor

enaess commented Jan 11, 2023

@yarda Thank you!

I think this is really up to @paulusmack, but I think it is clear now. You are asking to actually include the license as an actual text file. It can be named LICENSE, LICENSE.GPL-2.0 or LICENSE.BSD3WAY or whatever. Should be something that we can accommodate as a part of the distribution.

@enaess
Copy link
Contributor

enaess commented Mar 18, 2023

@rfc1036 The pppd-2.5.0 release is very close. We're waiting a few days to ensure stability and last minute alerts. Would you be able to check on the debian package and see if anything needs changes?

@rfc1036
Copy link
Contributor

rfc1036 commented Mar 18, 2023

This is way too late for the next Debian release, so no hurry.

I have not heard back from @bootc about my first set of changes, so I did no further work on the package.

@bootc
Copy link
Contributor

bootc commented Mar 18, 2023

Sorry @rfc1036 I did look at the changes and generally agree with what you did, but didn't go as far as pushing a package based on a snapshot as I wasn't sure of the value of it. During the freeze we wouldn't realistically be able to get the reverse dependencies (e.g. other packages supplying plugins) updated to match, and I suspect most users of ppp do it with some kind of out-of-tree plugin such as NetworkManager. I got the impression the 2.5.0 release wasn't far enough away and worth waiting for; and as you said there was no way of getting it into the Bookworm release anyway, and we can't upload to sid as that's best kept clear in case we need to upload something for bookworm before its release.

@rfc1036
Copy link
Contributor

rfc1036 commented Mar 18, 2023

We can upload to experimental, let me know your plans.

@enaess
Copy link
Contributor

enaess commented Mar 18, 2023

I totally get the reluctance to put ppp package into sid (or even Bookworm) that train has sailed. I was primarily hoping that @rfc1036 had a package recipe ready so there is no need to wait for that development.

I've already submitted upstream patches for many of the network-manager-plugins, and even for network-manager itself (pending merge, just need to figure out the meson build).

I think pptpd and one more project is needing patches, but it's on my radar.

@paulusmack
Copy link
Collaborator

2.5.0 is out now.

@Neustradamus
Copy link
Member

Dear all,

@paulusmack will prepare a 2.5.1 release build and ask some questions:

Can you look it?

Thanks in advance.

@Neustradamus
Copy link
Member

Neustradamus commented Sep 18, 2024

PPP 2.5.1 has been released (2024-09-18)

@Neustradamus
Copy link
Member

Neustradamus commented Oct 16, 2024

2024-10-16: PPP has been updated to 2.5.1 for:

ALT: @shabashaba:

Alpine: @ncopa:

Arch: @felixonmars:

Artix: @Dudemanguy:

Chimera: @nekopsykose:

CRUX: @TimB87:

Debian Experimental: @bootc:

Gentoo: @jkroonza and @floppym

KaOS: @demmm:

Liguros:

Manjaro (Unstable + Testing):

OpenMamba:

OpenWrt: @DragonBluep and @icegood:

Parabola:

Side: @4fury-c3440d8:

Slackware: @alienbob:

T2: @rxrbln:

Thanks to all.

Others, please check it, thanks in advance.

@Neustradamus
Copy link
Member

Dear all,

@paulusmack has planned a new build in few days, you have not patches?

@bootc, @rfc1036: What is the latest date to have a new PPP version (2.5.2) in Debian Testing before Debian 13 freeze?

@Neustradamus
Copy link
Member

PPP 2.5.2 has been released (2024-12-31)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

10 participants