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

Cosmetic fixes in configure.ac #592

Merged

Conversation

DimitriPapadopoulos
Copy link
Collaborator

Depends on #591.

* Reorganize order of operations in configure.ac in a single place.
* Consistent identation throughout the file.
* Minor changes in configure.ac documentation.
@DimitriPapadopoulos DimitriPapadopoulos changed the title WIP: Consistent indentation in configure.ac Cosmetic fixes in configure.ac Mar 23, 2020
Copy link
Collaborator

@mrbaseman mrbaseman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the fallback if neither ppp nor pppd are found on FreeBSD does not work anymore. However, this is already broken on the master branch, so it's an independent issue, which is easy to fix test "x$uname" = ... -> test "x$(uname)" = ...

Also, resolvconf generates additional output at configure time because output redirection does not work the same way as on linux &>/dev/null -> >/dev/null 2>/dev/null

I'll open another pull request for these FreeBSD issues.

@DimitriPapadopoulos
Copy link
Collaborator Author

DimitriPapadopoulos commented Mar 24, 2020

@mrbaseman OK, we have few changes in master for now so we can release a 1.13.2 with fixes for FreeBSD. Do you agree making this priority, before merging other pull requests like this one? This way we can avoid a 1.13 branch.

@mrbaseman mrbaseman mentioned this pull request Mar 24, 2020
@mrbaseman
Copy link
Collaborator

I would merge #592 #593 and #595 (all of them are really minor fixes or code cleanup) for the 1.13.2 and then continue working on #524 for instance

@DimitriPapadopoulos
Copy link
Collaborator Author

OK for merging them #592, #593, #595. Could you do that and then test on FreeBSD again?

By the way, could these fixes be of interest on macOS too? Homebrew checks for 1.13.1 have not completed yet so I don't know if there are build issues:
Homebrew/homebrew-core#52021

@DimitriPapadopoulos DimitriPapadopoulos merged commit 1ed4070 into adrienverge:master Mar 24, 2020
@DimitriPapadopoulos DimitriPapadopoulos deleted the configure branch March 24, 2020 11:46
@mrbaseman
Copy link
Collaborator

hmm... I have to rebase the other ones first. This time I have created new branches that are based on the other pull requests (in the hope that I don't have to rebase, but it didn't work).

The macOS issue was the same as on Fedora (as you have already mentioned in the Homebrew repo), but I thought it makes sense to repeat this statement here.

While writing this answer I have merged the other ones in the meantime, and I'll re-test on FreeBSD now (a colleague of mine was on-site this morning and has powered up the machine on which I have a couple of VMs, including the FreeBSD testing installation).

@mrbaseman
Copy link
Collaborator

I have tested the build system on FreeBSD again with #592, #593, #595 merged, and even if the ppp binary is renamed during configure time the fallback to the default path works fine. Also output redirection during the configure tests works now.

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

Successfully merging this pull request may close these issues.

2 participants