Hi,
Here's the summary of the IRC meeting.
---
COMMUNITY MEETING
Place: #openvpn-meeting on libera.chat
Date: Wed 27th April 2022
Time: 10:30 CEST (9:30 UTC)
Planned meeting topics for this meeting were here:
<https://community.openvpn.net/openvpn/wiki/Topics-2022-04-27>
Your local meeting time is easy to check from services such as
<http://www.timeanddate.com/worldclock>
SUMMARY
cron2, dazo, d12fk, djpig, lev, mattock, plaisthos participated in this
meeting.
---
Merged two openvpn-build PRs:
- https://github.com/OpenVPN/openvpn-build/pull/242
- https://github.com/OpenVPN/openvpn-build/pull/243
---
Talked about Uncrustify and noted that we should use 0.72 which has less
undesirable features (bugs) than 0.74. That said, even 0.72 needs some
tweaks to fix array block initializations (see example in meeting agenda
page).
We'd also need somebody to check if running uncrustify as part of "git
am" and "git rebase" is possible.
Also noted that GitHub Actions uses Uncrustify 0.69, which is old.
However, that will be solved when - hopefully in a few weeks - Ubuntu
22.04 images are available.
---
Talked about OpenVPN 2.6. Plaisthos' 28 patch series is being reviewed
by djpig and ordex. Good progress is being made.
--
Full chatlog attached
(11.27.36) mattock: almost meeting time
(11.27.58) ***dazo prepares
(11.28.52) plaisthos: moin moin
(11.29.48) cron2: hola
(11.30.14) cron2: mattock: your powershell script already got an ACK from lev
(11.31.17) mattock: +1, merged
(11.31.32) cron2: and the other one got approved and merged too
(11.31.38) mattock: yeah, great!
(11.31.41) cron2: very efficient meeting
(11.32.56) mattock: https://community.openvpn.net/openvpn/wiki/Topics-2022-04-27
(11.33.28) mattock: uncrustify?
(11.33.55) plaisthos: as you have probably all seen the big patchset for hmac
3way handshake/control channel improvement is on the mailing list
(11.34.15) cron2: wat, new patchset?
(11.34.21) ***cron2 hides
(11.34.31) cron2: where's dazo and d12fk...?
(11.34.48) ***dazo is here
(11.34.55) ***d12fk is here
(11.36.07) cron2: great
(11.36.09) dazo: I think plaisthos meant this one ....
https://patchwork.openvpn.net/project/openvpn2/list/?series=1532
(11.36.11) vpnHelper: Title: OpenVPN 2 - Patchwork (at patchwork.openvpn.net)
(11.36.16) dazo: which is already on the way into master
(11.36.24) cron2: dazo: I noticed :)
(11.36.36) cron2: so, uncrustify. I need dazo and d12fk's brains here :-)
(11.36.38) plaisthos: cron2: no, no new patchset yet
(11.37.16) cron2: we noticed that 0.72 does "what we have", and 0.74 introduces
some changes that do not look right, and are not consistent with itself (some
files get the changes, others not)
(11.37.20) cron2: so, 0.74 is buggy
(11.37.24) cron2: use 0.72
(11.38.14) cron2: that said, it still does "non-desirable" changes - dazo noted
that in the last commit - on array block initializations
(11.38.25) cron2: see example in the meeting topic
(11.38.59) cron2: it will move these blocks all the way to the left, unless you
move the opening bracket to "char foo[] = {"
(11.39.07) dazo: yupp
(11.39.10) cron2: but I do like the other style better, for this type of array
inits
(11.39.36) cron2: so, looking for a volunteer to go through the 473 zillion
options if there is something for "array initialization indent"...
(11.40.12) djpig [~flicht...@lovelace.lichtenheld.com] è entrato nella stanza.
(11.40.59) cron2: ... and then, another volunteer to see if there is a hook
that fires on "git am" or "git rebase" that we could use... the normal
pre-commit hook doesn't
(11.41.24) dazo: I believe our indent_brace is set to 0 by default. But that
needs to be tested. There's quite some 'indent_braces' group of options
(11.41.44) dazo: (I just quickly looked at uncrustify --config
dev-tools/uncrustify.conf --show-config)
(11.41.46) djpig: cron2: I can look into that
(11.42.21) lev__: hello
(11.43.14) cron2: dazo: for regular braces, that's what we want
(11.43.16) cron2: if(foo)
(11.43.17) cron2: {
(11.43.19) cron2: indent
(11.43.19) cron2: }
(11.43.28) dazo: yeah, true
(11.43.31) cron2: so some sort of other braces :-)
(11.43.37) ordex: aloha
(11.43.38) cron2: djpig: cool, thanks
(11.43.47) cron2: aloah lev__, ordex ;-)
(11.44.01) dazo: $ uncrustify --config dev-tools/uncrustify.conf --show-config
| grep -E ^indent_brace
(11.44.02) dazo: indent_brace = 0 # unsigned number
(11.44.02) dazo: indent_braces = false # true/false
(11.44.02) dazo: indent_braces_no_func = false # true/false
(11.44.02) dazo: indent_braces_no_class = false # true/false
(11.44.02) dazo: indent_braces_no_struct = false # true/false
(11.44.04) dazo: indent_brace_parent = false # true/false
(11.44.06) dazo: that's what we have to play with
(11.44.22) cron2: oh
(11.44.27) dazo: unless there is a different set for "curly braces"
(11.44.50) cron2: so indent_brace = 4, indent_braces = false,
indent_braces_no_func = true might "do the thing". Or not :-)
(11.45.33) mattock2 [~ya...@mobile-access-bcee70-29.dhcp.inet.fi] è entrato
nella stanza.
(11.45.35) cron2: there is MANY options with "brace"
(11.45.56) dazo: and it might mess up structs ....
(11.46.09) cron2: those we do not want to change ("they look good now")
(11.47.18) dazo: yup
(11.47.36) cron2: plaisthos wanted to bisect uncrustify 0.72->0.74 anyway to
see why it starts doing weird stuff... so we can then add a new option at it,
"openvpn_brace_style = true" that does all the things correctly
(11.47.48) dazo: hahaha
(11.47.53) ordex: :D
(11.47.57) ordex: that'd be nice
(11.47.59) ordex: and easier
(11.48.01) cron2: "this ist just a macro to set <75 options>" ;-)
(11.48.14) plaisthos: I am not adding code to uncrustify
(11.48.26) plaisthos: I have enough on my plate already :P
(11.48.34) cron2: just joking
(11.49.04) cron2: I think this might be a piece of software that will need
insane amounts of time to understand what happens, why
(11.49.08) dazo: plaisthos: just wait and see .... and you'll get nerd sniped
:-P
(11.49.38) cron2: nah, we can't afford that
(11.50.17) cron2: anyway, enough of uncrustify so far - I need to go to another
meeting in 10 minutes, so quick update on 2.6
(11.50.34) d12fk: for git am the pre-applypatchhook looks like it is usable
(11.51.01) cron2: plaisthos 28-part patch set is on the list. djpig has done a
good job in reviewing and testing about half of the patches, so we're up to
15/28 merged already
(11.51.18) cron2: ordex has 16 on his plate (I have nothing with a CRL right
now)
(11.51.29) ordex: yeah
(11.51.42) cron2: FreeBSD folks are finding DCO bugs \o/
(11.52.00) d12fk: and there's a pre-rebase hook as well
(11.52.33) cron2: d12fk: ah, very nice
(11.52.53) d12fk: "The default pre-applypatch hook, when enabled, runs the
pre-commit hook, if the latter is enabled."
(11.53.06) d12fk: so setting up the default is sufficient
(11.53.29) d12fk: for rebase it is something different, since we're in a
different situation there
(11.54.02) cron2: I'll play around with that (pre-applypatch). Maybe change it
to "only print warning, not fail" so I can fix the stuff then
(11.54.56) plaisthos: you can also do --no-verify on commit to skip the hooks
(11.55.16) d12fk: but that's cheating =)
(11.56.03) cron2: yeah, will find a workflow that will avoid the Big
Reformatting patches in future
(11.56.28) d12fk: what is the rebase use case for you?
(11.57.35) cron2: rebase is more plaisthos
(11.57.57) cron2: I only rebase if dazo commits something to the external
master repos, and I need to take over again
(11.58.10) cron2: (so, pull in sf/gitlab, rebase my tree on that, push to
sf/github/gitlab again)
(11.58.27) plaisthos: yeah. For now I might have a missed a change in one of
the 28 commits since I had to basically do that manually
(11.58.39) plaisthos: but in the future it should be easier
(11.58.51) djpig: yeah, the rebase hooks work completely different, so that
would be more work, while enabling pre-applypatch is trivial
(11.59.23) dazo: cron2: should we do a reformatting on release/2.5 too?
(11.59.25) djpig: should we patch precommit install to handle pre-applypatch?
(12.00.26) ***cron2 is out "next meeting" - sorry
(12.01.11) d12fk: do we have more to meet on, or is this the end
(12.01.30) mattock2: I don't have anything.
(12.02.07) d12fk: i think it would be reasonable to copy the default
pre-applypatch hook when we install our pre-commit
(12.02.26) d12fk: but then it is probably only "the mergers" who care
(12.04.24) djpig: hmm, as a reviewer it would also be helpful when reviewing
patches from people that do not have the hook, so that we can give early
feedback
(12.05.06) d12fk: okay fair point
(12.06.38) plaisthos: I have some github actions code for that
(12.06.47) plaisthos: but ubuntu 20 has 0.69
(12.07.29) plaisthos: so I want to wait until github releases 22.04 images to
avoid workarounds for getting a new version on ubuntu 20
(12.07.58) d12fk: 22.04 has 0.72
(12.08.04) d12fk: could be run in docker
(12.08.21) d12fk: until -latest is 22.04
(12.08.44) plaisthos: yeah
(12.08.47) plaisthos: but it is not that urgent
(12.09.04) d12fk: its a on-liner, so no big hassle
(12.09.09) d12fk: *one
(12.14.54) d12fk: where is the yaml plaisthos
(12.15.50) plaisthos: on my wip branch
(12.16.21) plaisthos: but as I said, we can just wait a week or two
(12.17.07) d12fk: do you think it'll be so fast?
(12.17.13) plaisthos: yeah
(12.17.36) plaisthos: read in a issue that they planning to release it soon
(12.17.38) d12fk: kk, waiting then
(12.19.21) ***d12fk heads out, waving
(12.24.05) ordex: bye
(12.24.10) dazo: o/
(12.41.07) ***cron2 waves too :-)
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
