Another fix related to unit test framework
PATCH 1/1 Another fix related to unit test framework
From: David Sommerseth <[email protected]...>
Continuing to fix breakage caused by commit 40cb4cfc5d011102.
It seems it was a conflict in vendor/Makefile.am's distdir target, confusing autotools so it wouldn't actually parse that directory properly. The result was that 'make distcheck' would fail and tarballs created would just ship with an empty vendor/ directory.
Also remove the 'foreign' AUTOMAKE_OPTIONS flag, as we don't use that many places at all. Things work well without this flag.
--- vendor/Makefile.am | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-)
vendor/Makefile.am +2 -7
@@ -1,5 +1,3 @@ -AUTOMAKE_OPTIONS = foreign - cmockasrc = @VENDOR_SRC_ROOT@/cmocka # needs an absolute path bc. of the cmake invocation cmockabuild = @VENDOR_BUILD_ROOT@/cmocka cmockainstall = @VENDOR_DIST_ROOT@
@@ -10,12 +8,9 @@ MAINTAINERCLEANFILES = \ $(cmockainstall) \ @VENDOR_BUILD_ROOT@ -distdir: - mkdir -p $(cmockainstall) - -libcmocka: distdir - mkdir -p $(cmockabuild) +libcmocka: if CMOCKA_INITIALIZED + mkdir -p $(cmockabuild) $(cmockainstall) (cd $(cmockabuild) && cmake -DCMAKE_INSTALL_PREFIX=$(cmockainstall) $(cmockasrc) && make && make install) endif
Signature · 1 line
1.8.3.1
Hi,
vendor/Makefile.am MAINTAINERCLEANFILES = \ · line 13
Quoted 2 lines · David Sommerseth
if
CMOCKA_INITIALIZEDvendor/Makefile.am +1
-0+ mkdir -p $(cmockabuild) $(cmockainstall)
This will still blow up if the source or build directory contain whitespace
- if absolute paths outside Makefile's control are used, they must be quoted.
gert
Signature · 4 lines
USENET is *not* the non-clickable part of WWW!
//www.muc.de/~gert/
Gert Doering - Munich, Germany [email protected]...
fax: +49-89-35655025 [email protected]...This patch seems to do what it promises, so a feature-ACK from me.
Signature · 5 lines
Samuli Seppänen Community Manager OpenVPN Technologies, Inc irc freenode net: mattock
Quoted 43 lines
From: David Sommerseth <[email protected]...>
Continuing to fix breakage caused by commit
40cb4cfc5d011102.It seems it was a conflict in vendor/Makefile.am's distdir target, confusing autotools so it wouldn't actually parse that directory properly. The result was that 'make distcheck' would fail and tarballs created would just ship with an empty vendor/ directory.
Also remove the 'foreign'
AUTOMAKE_OPTIONSflag, as we don't use that many places at all. Things work well without this flag.Signed-off-by David Sommerseth <[email protected]...>--- vendor/Makefile.am | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-)vendor/Makefile.am +2
-7@@ -1,5 +1,3 @@ -AUTOMAKE_OPTIONS = foreign - cmockasrc = @VENDOR_SRC_ROOT@/cmocka # needs an absolute path bc. of the cmake invocation cmockabuild = @VENDOR_BUILD_ROOT@/cmocka cmockainstall = @VENDOR_DIST_ROOT@@@ -10,12 +8,9 @@ MAINTAINERCLEANFILES = \ $(cmockainstall) \ @VENDOR_BUILD_ROOT@ -distdir: - mkdir -p $(cmockainstall) - -libcmocka: distdir - mkdir -p $(cmockabuild) +libcmocka: if CMOCKA_INITIALIZED + mkdir -p $(cmockabuild) $(cmockainstall) (cd $(cmockabuild) && cmake -DCMAKE_INSTALL_PREFIX=$(cmockainstall) $(cmockasrc) && make && make install) endif
PATCH v2 1/1 Another fix related to unit test framework
In reply to David SommersethFrom: David Sommerseth <[email protected]...>
Continuing to fix breakage caused by commit 40cb4cfc5d011102.
It seems it was a conflict in vendor/Makefile.am's distdir target, confusing autotools so it wouldn't actually parse that directory properly. The result was that 'make distcheck' would fail and tarballs created would just ship with an empty vendor/ directory.
Also remove the 'foreign' AUTOMAKE_OPTIONS flag, as we don't use that many places at all. Things work well without this flag.
The comment had to be moved to a single line, otherwise the white spaces between the end of the variable assignment and the hash character got added to the variable.
[v2 - Fix white space issues in path variables]
--- vendor/Makefile.am | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-)
vendor/Makefile.am +5 -9
@@ -1,6 +1,5 @@ -AUTOMAKE_OPTIONS = foreign - -cmockasrc = @VENDOR_SRC_ROOT@/cmocka # needs an absolute path bc. of the cmake invocation +# needs an absolute path bc. of the cmake invocation +cmockasrc = @VENDOR_SRC_ROOT@/cmocka cmockabuild = @VENDOR_BUILD_ROOT@/cmocka cmockainstall = @VENDOR_DIST_ROOT@
@@ -10,13 +9,10 @@ MAINTAINERCLEANFILES = \ $(cmockainstall) \ @VENDOR_BUILD_ROOT@ -distdir: - mkdir -p $(cmockainstall) - -libcmocka: distdir - mkdir -p $(cmockabuild) +libcmocka: if CMOCKA_INITIALIZED - (cd $(cmockabuild) && cmake -DCMAKE_INSTALL_PREFIX=$(cmockainstall) $(cmockasrc) && make && make install) + mkdir -p "$(cmockabuild)" "$(cmockainstall)" + (cd "$(cmockabuild)" && cmake -DCMAKE_INSTALL_PREFIX="$(cmockainstall)" "$(cmockasrc)" && make && make install) endif check: libcmocka
Signature · 1 line
1.8.3.1
PATCH 1/1 Another fix related to unit test framework
In reply to David SommersethFrom: David Sommerseth <[email protected]...>
Continuing to fix breakage caused by commit 40cb4cfc5d011102.
It seems it was a conflict in vendor/Makefile.am's distdir target, confusing autotools so it wouldn't actually parse that directory properly. The result was that 'make distcheck' would fail and tarballs created would just ship with an empty vendor/ directory.
Also remove the 'foreign' AUTOMAKE_OPTIONS flag, as we don't use that many places at all. Things work well without this flag.
The comment had to be moved to a single line, otherwise the white spaces between the end of the variable assignment and the hash character got added to the variable.
[v3 - Further improve white space issues, now 'make clean'
should work too]
[v2 - Fix white space issues in path variables]
--- vendor/Makefile.am | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-)
vendor/Makefile.am +9 -13
@@ -1,21 +1,17 @@ -AUTOMAKE_OPTIONS = foreign - -cmockasrc = @VENDOR_SRC_ROOT@/cmocka # needs an absolute path bc. of the cmake invocation -cmockabuild = @VENDOR_BUILD_ROOT@/cmocka -cmockainstall = @VENDOR_DIST_ROOT@ +# needs an absolute path bc. of the cmake invocation +cmockasrc = "@VENDOR_SRC_ROOT@/cmocka" +cmockabuild = "@VENDOR_BUILD_ROOT@/cmocka" +cmockainstall = "@VENDOR_DIST_ROOT@" MAINTAINERCLEANFILES = \ $(srcdir)/Makefile.in \ - $(cmockabuild) \ - $(cmockainstall) \ - @VENDOR_BUILD_ROOT@ - -distdir: - mkdir -p $(cmockainstall) + "$(cmockabuild)" \ + "$(cmockainstall)" \ + "@VENDOR_BUILD_ROOT@" -libcmocka: distdir - mkdir -p $(cmockabuild) +libcmocka: if CMOCKA_INITIALIZED + mkdir -p $(cmockabuild) $(cmockainstall) (cd $(cmockabuild) && cmake -DCMAKE_INSTALL_PREFIX=$(cmockainstall) $(cmockasrc) && make && make install) endif
Signature · 1 line
1.8.3.1
Hi,
Quoted 3 lines · David Sommerseth
From: David Sommerseth <[email protected]...>
Continuing to fix breakage caused by commit
40cb4cfc5d011102.
[..]
Not having actually tested this, looking at it seems to properly quote the bits that caused buildbot explosion and "make dist" fails (also as discussed between David and Samuli on IRC).
So - ACK! Please apply and let's make the buildbots all green again :-)
gert
Signature · 4 lines
USENET is *not* the non-clickable part of WWW!
//www.muc.de/~gert/
Gert Doering - Munich, Germany [email protected]...
fax: +49-89-35655025 [email protected]...Quoted 11 lines · Gert Doering
Quoted 3 lines · David Sommerseth
From: David Sommerseth <[email protected]...>
Continuing to fix breakage caused by commit
40cb4cfc5d011102.[..]
Not having actually tested this, looking at it seems to properly quote the bits that caused buildbot explosion and "make dist" fails (also as discussed between David and Samuli on IRC).
So - ACK! Please apply and let's make the buildbots all green again :-)
Don't know about bots, at least one human cross-compiling for windows is happy now..
Selva
Quoted 13 lines · Selva Nair
Quoted 10 lines · Gert Doering
Quoted 3 lines · David Sommerseth
From: David Sommerseth <[email protected]...>
Continuing to fix breakage caused by commit
40cb4cfc5d011102.[..]
Not having actually tested this, looking at it seems to properly quote the bits that caused buildbot explosion and "make dist" fails (also as discussed between David and Samuli on IRC).
Quoted 6 lines
Quoted 1 line
So - ACK! Please apply and let's make the buildbots all green again :-)
Don't know about bots, at least one human cross-compiling for windows is happy now..
The joy was short-lived. mingw cross-compile from release/2.3 tarball is also broken:
error: cannot find input file: `vendor/Makefile.in'
Thankfully, cherry-pick 41ab12f06253c fixes it.
IMO, the unit testing patches shouldn't have been merged into the release branch
Selva
Quoted 3 lines
Selva
…
Quoted 1 line
IMO, the unit testing patches shouldn't have been merged into the release branch
I agree. This patch was in retrospective clearly not ready for a release branch. A lot of people spend time to hot fix a broken build.
My root cause analysis boils down to:
Developers cannot detect multi-platform (build/run) issues with an appropriate effort (or at all).
We already have discussed solutions to this:
1) Enable developers to run the “authoritative” buildslave tests before submitting patches
2) Provide developers tooling to quickly (preferred: locally) run iterations while developing
#1 will need some processes & tooling by the current maintainers. #2 will be taken care of with the Vagrant based integration tests.
Opinions?
Jens
Quoted 6 lines
…
Quoted 2 lines
IMO, the unit testing patches shouldn't have been merged into the release branch
Quoted 7 lines
I agree. This patch was in retrospective clearly not ready for a release branch. A lot of people spend time to hot fix a broken build.
My root cause analysis boils down to:
Developers cannot detect multi-platform (build/run) issues with an
appropriate effort (or at all).
This is probably correct: the codebase is complex enough to cause breakage on many types of changes, no matter how carefully the code is reviewed. This is often because of the sheer number of options and their invisible interplay. A recent example is the recent persist-tun / WFP filtering issue, which slipped through all testing and review.
Regression testing using unit tests could possibly help us prevent this type of breakages.
Quoted 4 lines
We already have discussed solutions to this:
1) Enable developers to run the “authoritative” buildslave tests before submitting patches
The buildmaster can trigger manual builds from a different branch (e.g. "staging/somebody"). Access to the buildmaster can be granted selectively to trusted core developers. Non-trivial patches from other people should probably be tested by one of these core developers before pushing them to "master". Or we can just accept the fact that "master" might be broken occasionally and fix the problems promptly.
Quoted 2 lines
2) Provide developers tooling to quickly (preferred: locally) run iterations while developing
The Vagrant approach is nice because it will eventually allow developers to run a fairly extensive smoketests on several various operating systems with minimal effort:
<https://github.com/OpenVPN/openvpn/pull/45>
Right now the OS coverage is fairly minimal, but more variants can be added easily, and we're not limited to Linux/Ubuntu like with Travis.
Quoted 4 lines
#1 will need some processes & tooling by the current maintainers. #2 will be taken care of with the Vagrant based integration tests.
Opinions?
In addition to #1 and #2 we have Travis-CI smoke-testing in the pipeline:
<https://github.com/OpenVPN/openvpn/pull/52>
Travis-CI gives us some confidence on the quality of GitHub PRs, but it's test matrix is much narrower than that of Buildbot, so it's definitely not a panacea.
The only real way to test the more tricky corner-cases is to make it as easy as possible to use Git "master" -based builds. For Windows this is already covered by the Windows "buildslave" and the snapshots it generates. I can setup apt repositories with Debian/Ubuntu builds based on Git "master", but right now I don't have enough time to commit to that.
Signature · 5 lines
Samuli Seppänen Community Manager OpenVPN Technologies, Inc irc freenode net: mattock
it is not true that Travis-CI is limited to Linux/Ubuntu, at least there's Mac OS X.
and we can set up (later) cross builds for MIPS/ARM/Windows/whatever (not sure about "make check")
cross build would be good starting point, if there was such thing already, we could notice that mingw build got broken
2016-06-07 12:30 GMT+05:00 Samuli Seppänen <[email protected]...>:
Quoted 87 lines
Quoted 6 lines
…
Quoted 2 lines
IMO, the unit testing patches shouldn't have been merged into the release branch
Quoted 7 lines
I agree. This patch was in retrospective clearly not ready for a release branch. A lot of people spend time to hot fix a broken build.
My root cause analysis boils down to:
Developers cannot detect multi-platform (build/run) issues with an
appropriate effort (or at all).This is probably correct: the codebase is complex enough to cause breakage on many types of changes, no matter how carefully the code is reviewed. This is often because of the sheer number of options and their invisible interplay. A recent example is the recent persist-tun / WFP filtering issue, which slipped through all testing and review.
Regression testing using unit tests could possibly help us prevent this type of breakages.
Quoted 4 lines
We already have discussed solutions to this:
1) Enable developers to run the “authoritative” buildslave tests before submitting patches
The buildmaster can trigger manual builds from a different branch (e.g. "staging/somebody"). Access to the buildmaster can be granted selectively to trusted core developers. Non-trivial patches from other people should probably be tested by one of these core developers before pushing them to "master". Or we can just accept the fact that "master" might be broken occasionally and fix the problems promptly.
Quoted 2 lines
2) Provide developers tooling to quickly (preferred: locally) run iterations while developing
The Vagrant approach is nice because it will eventually allow developers to run a fairly extensive smoketests on several various operating systems with minimal effort:
<https://github.com/OpenVPN/openvpn/pull/45>
Right now the OS coverage is fairly minimal, but more variants can be added easily, and we're not limited to Linux/Ubuntu like with Travis.
Quoted 4 lines
#1 will need some processes & tooling by the current maintainers. #2 will be taken care of with the Vagrant based integration tests.
Opinions?
In addition to #1 and #2 we have Travis-CI smoke-testing in the pipeline:
<https://github.com/OpenVPN/openvpn/pull/52>
Travis-CI gives us some confidence on the quality of GitHub PRs, but it's test matrix is much narrower than that of Buildbot, so it's definitely not a panacea.
The only real way to test the more tricky corner-cases is to make it as easy as possible to use Git "master" -based builds. For Windows this is already covered by the Windows "buildslave" and the snapshots it generates. I can setup apt repositories with Debian/Ubuntu builds based on Git "master", but right now I don't have enough time to commit to that.
--
Samuli Seppänen
Community Manager
OpenVPN Technologies, Incirc freenode net: mattock
------------------------------------------------------------------------------
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are
consuming the most bandwidth. Provides multi-vendor support for NetFlow, J-Flow, sFlow and other flows. Make informed decisions using capacity planning reports. https://ad.doubleclick.net/ddm/clk/305295220;132659582;e
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
I stand corrected. However, cross-building is not a replacement for building on the actual OS.
Do cross-builds generally catch useful issues, or do they tend to catch issues related to the cross-building environment itself?
Signature · 5 lines
Samuli Seppänen Community Manager OpenVPN Technologies, Inc irc freenode net: mattock
Quoted 104 lines
it is not true that Travis-CI is limited to Linux/Ubuntu, at least there's Mac OS X.
and we can set up (later) cross builds for MIPS/ARM/Windows/whatever (not sure about "make check")cross build would be good starting point, if there was such thing already, we could notice that mingw build got broken
2016-06-07 12:30 GMT+05:00 Samuli Seppänen <[email protected]... <mailto:[email protected]...>>:
Quoted 6 lines
…
Quoted 2 lines
IMO, the unit testing patches shouldn't have been merged into the release branch
I agree. This patch was in retrospective clearly not ready for a
release
> branch. A lot of people spend time to hot fix a broken build. >
> My root cause analysis boils down to:
>
> Developers cannot detect multi-platform (build/run) issues with an
> appropriate effort (or at all).This is probably correct: the codebase is complex enough to cause breakage on many types of changes, no matter how carefully the code is reviewed. This is often because of the sheer number of options and their invisible interplay. A recent example is the recent persist-tun / WFP filtering issue, which slipped through all testing and review.
Regression testing using unit tests could possibly help us prevent this type of breakages.
> We already have discussed solutions to this:
>
> 1) Enable developers to run the “authoritative” buildslave tests before
> submitting patchesThe buildmaster can trigger manual builds from a different branch (e.g. "staging/somebody"). Access to the buildmaster can be granted selectively to trusted core developers. Non-trivial patches from other people should probably be tested by one of these core developers before pushing them to "master". Or we can just accept the fact that "master" might be broken occasionally and fix the problems promptly.
> 2) Provide developers tooling to quickly (preferred: locally) run > iterations while developing
The Vagrant approach is nice because it will eventually allow developers to run a fairly extensive smoketests on several various operating systems with minimal effort:
<https://github.com/OpenVPN/openvpn/pull/45>
Right now the OS coverage is fairly minimal, but more variants can be added easily, and we're not limited to Linux/Ubuntu like with Travis.
> #1 will need some processes & tooling by the current maintainers. > #2 will be taken care of with the Vagrant based integration tests. >
> Opinions?In addition to #1 and #2 we have Travis-CI smoke-testing in the pipeline:
<https://github.com/OpenVPN/openvpn/pull/52>
Travis-CI gives us some confidence on the quality of GitHub PRs, but it's test matrix is much narrower than that of Buildbot, so it's definitely not a panacea.
The only real way to test the more tricky corner-cases is to make it as easy as possible to use Git "master" -based builds. For Windows this is already covered by the Windows "buildslave" and the snapshots it generates. I can setup apt repositories with Debian/Ubuntu builds based on Git "master", but right now I don't have enough time to commit to that.
--
Samuli Seppänen
Community Manager
OpenVPN Technologies, Incirc freenode net: mattock
------------------------------------------------------------------------------
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are
consuming the most bandwidth. Provides multi-vendor support for NetFlow, J-Flow, sFlow and other flows. Make informed decisions using capacity planning reports.
https://ad.doubleclick.net/ddm/clk/305295220;132659582;e
_______________________________________________
Openvpn-devel mailing list [email protected] <mailto:[email protected]>
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Hi,
Quoted 5 lines · Samuli Seppänen
I stand corrected. However, cross-building is not a replacement for building on the actual OS.
Do cross-builds generally catch useful issues, or do they tend to catch issues related to the cross-building environment itself?
The mingw builds that you run have caught quite a few commits with unexpected side effects (header files not right for windows, and such stuff) - so they are definitely useful.
Occasionaly, they catch "building environment bugs", which is annoying, but easier than setting up windows build environments...
What is missing with the cross-builds is to be able to *run* the binary to do full-binary tests (and, obviously, unit tests also do not work with cross-built stuff). So it's not as good as automated local builds with test suite, but until we can have that for windows, I'm happy with mingw cross-builds and automatically uploaded snapshots for all commits :-)
gert
Signature · 4 lines
USENET is *not* the non-clickable part of WWW!
//www.muc.de/~gert/
Gert Doering - Munich, Germany [email protected]...
fax: +49-89-35655025 [email protected]...mingw is an official way of building windows packages. I guess something like that appliable for ARM as well (I haven't heard about compilers running on those machines)
so, if we can catch an issue during such compile, it is good.
2016-06-07 12:58 GMT+05:00 Samuli Seppänen <[email protected]...>:
Quoted 132 lines
I stand corrected. However, cross-building is not a replacement for building on the actual OS.
Do cross-builds generally catch useful issues, or do they tend to catch issues related to the cross-building environment itself?
--
Samuli Seppänen
Community Manager
OpenVPN Technologies, Incirc freenode net: mattock
it is not true that Travis-CI is limited to Linux/Ubuntu, at least
Quoted 116 lines
there's Mac OS X.
and we can set up (later) cross builds for MIPS/ARM/Windows/whatever (not sure about "make check")cross build would be good starting point, if there was such thing already, we could notice that mingw build got broken
2016-06-07 12:30 GMT+05:00 Samuli Seppänen <[email protected]... <mailto:[email protected]...>>:
Quoted 6 lines
…
Quoted 2 lines
IMO, the unit testing patches shouldn't have been merged into the release branch
I agree. This patch was in retrospective clearly not ready for a
release
> branch. A lot of people spend time to hot fix a broken build. >
> My root cause analysis boils down to:
>
> Developers cannot detect multi-platform (build/run) issues with
anQuoted 1 line
appropriate effort (or at all).
This is probably correct: the codebase is complex enough to cause breakage on many types of changes, no matter how carefully the code is reviewed. This is often because of the sheer number of options and
their
invisible interplay. A recent example is the recent persist-tun / WFP filtering issue, which slipped through all testing and review.Regression testing using unit tests could possibly help us prevent
this
type of breakages.> We already have discussed solutions to this:
>
> 1) Enable developers to run the “authoritative” buildslave tests before
> submitting patchesThe buildmaster can trigger manual builds from a different branch
(e.g.
"staging/somebody"). Access to the buildmaster can be granted selectively to trusted core developers. Non-trivial patches from other people should probably be tested by one of these core developers
before
pushing them to "master". Or we can just accept the fact that "master" might be broken occasionally and fix the problems promptly.> 2) Provide developers tooling to quickly (preferred: locally) run > iterations while developing
The Vagrant approach is nice because it will eventually allow
developers
to run a fairly extensive smoketests on several various operating systems with minimal effort:<https://github.com/OpenVPN/openvpn/pull/45>
Right now the OS coverage is fairly minimal, but more variants can be added easily, and we're not limited to Linux/Ubuntu like with Travis.
> #1 will need some processes & tooling by the current maintainers. > #2 will be taken care of with the Vagrant based integration tests. >
> Opinions?In addition to #1 and #2 we have Travis-CI smoke-testing in the pipeline:
<https://github.com/OpenVPN/openvpn/pull/52>
Travis-CI gives us some confidence on the quality of GitHub PRs, but it's test matrix is much narrower than that of Buildbot, so it's definitely not a panacea.
The only real way to test the more tricky corner-cases is to make it
as
easy as possible to use Git "master" -based builds. For Windows this
is
already covered by the Windows "buildslave" and the snapshots it generates. I can setup apt repositories with Debian/Ubuntu builds
based
on Git "master", but right now I don't have enough time to commit to that.--
Samuli Seppänen
Community Manager
OpenVPN Technologies, Incirc freenode net: mattock
------------------------------------------------------------------------------
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are
consuming the most bandwidth. Provides multi-vendor support for
NetFlow,
J-Flow, sFlow and other flows. Make informed decisions using capacity planning reports.
https://ad.doubleclick.net/ddm/clk/305295220;132659582;e
_______________________________________________
Openvpn-devel mailing list [email protected] <mailto:[email protected]>
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Am 07.06.16 um 10:23 schrieb Илья Шипицин:
Quoted 4 lines
mingw is an official way of building windows packages. I guess something like that appliable for ARM as well (I haven't heard about compilers running on those machines)
Raspberry pi!
Arne
oops, I was wrong.
2016-06-07 13:28 GMT+05:00 Arne Schwabe <[email protected]...>:
Quoted 11 lines
Am 07.06.16 um 10:23 schrieb Илья Шипицин:
Quoted 4 lines
mingw is an official way of building windows packages. I guess something like that appliable for ARM as well (I haven't heard about compilers running on those machines)
Raspberry pi!
Arne
hi,
Quoted 3 lines · ???????? ??????????????
mingw is an official way of building windows packages. I guess something like that appliable for ARM as well (I haven't heard about compilers running on those machines)
$ uname -mo armv5tel GNU/Linux $ gcc -v Using built-in specs. Target: arm-linux-gnueabi
of course you'd typically not compile on a slow ARM system, but if it's just a few lines of code, getting a cross-build environment up and running is way more effort than "just install the compiler and compile locally"... if disk space permits.
gert
Signature · 4 lines
USENET is *not* the non-clickable part of WWW!
//www.muc.de/~gert/
Gert Doering - Munich, Germany [email protected]...
fax: +49-89-35655025 [email protected]...Quoted 14 lines
hi,
Quoted 3 lines · ???????? ??????????????
mingw is an official way of building windows packages. I guess something like that appliable for ARM as well (I haven't heard about compilers running on those machines)
$ uname -mo armv5tel GNU/Linux $ gcc -v Using built-in specs. Target: arm-linux-gnueabi
Quoted 4 lines
of course you'd typically not compile on a slow ARM system, but if it's just a few lines of code, getting a cross-build environment up and running is way more effort than "just install the compiler and compile locally"... if disk space permits.
People do use openvpn-build to cross-compile for ARM.
Signature · 5 lines
Samuli Seppänen Community Manager OpenVPN Technologies, Inc irc freenode net: mattock
Hi,
Quoted 8 lines · Samuli Seppänen
This is probably correct: the codebase is complex enough to cause breakage on many types of changes, no matter how carefully the code is reviewed. This is often because of the sheer number of options and their invisible interplay. A recent example is the recent persist-tun / WFP filtering issue, which slipped through all testing and review.
Regression testing using unit tests could possibly help us prevent this type of breakages.
In particular for *that* example: no way unit testing would have found that - "full connection testing with multiple remotes and tunnel connection issues on the first tunnel" would have been needed.
Unit testing might have found the issue that on closing of the WFP in master, the handle wasn't NULLed, so a second close call would have caused a crash - if someone had thought of adding a test case for "call close twice!" *and* ran this on windows.
Unit testing is good, but has no chance to find "full program test" failures in certain combinations of options and remotes - which is what t_client is there for, but even that one wouldn't have caught the WFP thing as it's fairly hard to test for that.
Quoted 11 lines
Quoted 4 lines
We already have discussed solutions to this:
1) Enable developers to run the ?authoritative? buildslave tests before submitting patches
The buildmaster can trigger manual builds from a different branch (e.g. "staging/somebody"). Access to the buildmaster can be granted selectively to trusted core developers. Non-trivial patches from other people should probably be tested by one of these core developers before pushing them to "master". Or we can just accept the fact that "master" might be broken occasionally and fix the problems promptly.
Haven't we been here before?
- add testing branches to the github repo for (semi-)trusted developers
- teach buildbot to use the github repo
- enable (semi-)trusted developers to push their "I want this tested!" code to github/openvpn/testing-<user>/ and have buildbot test it
we can't open this to the world, as the t_client tests need sudo privileges, so anyone who can push a patch to a testing tree can run arbitrary commands on the buildslaves ("just build whatever you want into something called 'openvpn' and run that with sudo from t_client") - so, (semi-)trusted developers only.
(This *should* have been discussed last monday, but it slipped off the agenda)
Quoted 11 lines
Quoted 2 lines
2) Provide developers tooling to quickly (preferred: locally) run iterations while developing
The Vagrant approach is nice because it will eventually allow developers to run a fairly extensive smoketests on several various operating systems with minimal effort:
<https://github.com/OpenVPN/openvpn/pull/45>
Right now the OS coverage is fairly minimal, but more variants can be added easily, and we're not limited to Linux/Ubuntu like with Travis.
I'm not seeing who is going to maintain all these VMs so every developer can run the full suite across all OSes...
gert
Signature · 4 lines
USENET is *not* the non-clickable part of WWW!
//www.muc.de/~gert/
Gert Doering - Munich, Germany [email protected]...
fax: +49-89-35655025 [email protected]...Quoted 39 lines
Hi,
Quoted 8 lines · Samuli Seppänen
This is probably correct: the codebase is complex enough to cause breakage on many types of changes, no matter how carefully the code is reviewed. This is often because of the sheer number of options and their invisible interplay. A recent example is the recent persist-tun / WFP filtering issue, which slipped through all testing and review.
Regression testing using unit tests could possibly help us prevent this type of breakages.
In particular for *that* example: no way unit testing would have found that - "full connection testing with multiple remotes and tunnel connection issues on the first tunnel" would have been needed.
Unit testing might have found the issue that on closing of the WFP in master, the handle wasn't NULLed, so a second close call would have caused a crash - if someone had thought of adding a test case for "call close twice!" *and* ran this on windows.
Unit testing is good, but has no chance to find "full program test" failures in certain combinations of options and remotes - which is what
t_clientis there for, but even that one wouldn't have caught the WFP thing as it's fairly hard to test for that.Quoted 11 lines
Quoted 4 lines
We already have discussed solutions to this:
1) Enable developers to run the ?authoritative? buildslave tests before submitting patches
The buildmaster can trigger manual builds from a different branch (e.g. "staging/somebody"). Access to the buildmaster can be granted selectively to trusted core developers. Non-trivial patches from other people should probably be tested by one of these core developers before pushing them to "master". Or we can just accept the fact that "master" might be broken occasionally and fix the problems promptly.
Quoted 6 lines
Haven't we been here before?
- add testing branches to the github repo for (semi-)trusted developers
- teach buildbot to use the github repo
- enable (semi-)trusted developers to push their "I want this tested!" code to github/openvpn/testing-<user>/ and have buildbot test it
Yes, exactly, this is what I was describing above in other words :).
Quoted 5 lines
we can't open this to the world, as the
t_clienttests need sudo privileges, so anyone who can push a patch to a testing tree can run arbitrary commands on the buildslaves ("just build whatever you want into something called 'openvpn' and run that with sudo fromt_client") - so, (semi-)trusted developers only.
This is not entirely true, because the build steps are hardcoded. However, I would definitely not open this to the world, because there is plenty of room for misuse, and Buildbot might have security issues which could be exploited.
Quoted 13 lines
(This *should* have been discussed last monday, but it slipped off the agenda)
Quoted 9 lines
Quoted 2 lines
2) Provide developers tooling to quickly (preferred: locally) run iterations while developing
The Vagrant approach is nice because it will eventually allow developers to run a fairly extensive smoketests on several various operating systems with minimal effort:
Quoted 7 lines
Quoted 2 lines
Right now the OS coverage is fairly minimal, but more variants can be added easily, and we're not limited to Linux/Ubuntu like with Travis.
I'm not seeing who is going to maintain all these VMs so every developer can run the full suite across all OSes...
gert
It seems that a summary of how Vagrant operates is in order here.
Vagrant uses pre-built images as a starting point. These images do not (and should not) be built by OpenVPN developers. The only things _we_ have to maintain are the Vagrant files, which are basically recipies for configuring the base boxes into an OpenVPN test VMs.
So, when a developer wants to run the integration tests this is what happens:
- Vagrant fetches the pre-built VM images from a remote source
- The image is launched into Virtualbox (or other virtualization system)
- Vagrant runs the initialization scripts in the Vagrantfile
- The system is ready to use and stored for future use
The initialization scripts can automatically run integration tests when the VM launches, or that can be left to the developer.
When the developer wants to rerun the tests, he/she can just launch the VM with "vagrant up" and SSH in with "vagrant ssh". When the Vagrant file has changed significantly, the old box can be nuked and a new one rebuilt in <5 minutes.
Summary: very little maintenance is required for Vagrant. It is not like buildbot, where we actually have to build the VMs from scratch.
Signature · 5 lines
Samuli Seppänen Community Manager OpenVPN Technologies, Inc irc freenode net: mattock
Hi,
Quoted 7 lines · Samuli Seppänen
Quoted 5 lines
we can't open this to the world, as the
t_clienttests need sudo privileges, so anyone who can push a patch to a testing tree can run arbitrary commands on the buildslaves ("just build whatever you want into something called 'openvpn' and run that with sudo fromt_client") - so, (semi-)trusted developers only.This is not entirely true, because the build steps are hardcoded.
Trivial :-) - just add a patch that will
cp evilscript.sh src/openvpn/openvpn
at the end of the build phase.
Then, run "make check", and t_client.sh will happily execute
sudo src/openvpn/openvpn $options
which now runs "evilscript.sh" with full root access...
Quoted 3 lines
However, I would definitely not open this to the world, because there is plenty of room for misuse, and Buildbot might have security issues which could be exploited.
Ideed :-)
[..]
Quoted 7 lines
It seems that a summary of how Vagrant operates is in order here.
Vagrant uses pre-built images as a starting point. These images do not (and should not) be built by OpenVPN developers. The only things _we_ have to maintain are the Vagrant files, which are basically recipies for configuring the base boxes into an OpenVPN test VMs.
Quoted 7 lines
So, when a developer wants to run the integration tests this is what happens:
- Vagrant fetches the pre-built VM images from a remote source
- The image is launched into Virtualbox (or other virtualization system)
- Vagrant runs the initialization scripts in the Vagrantfile
- The system is ready to use and stored for future use
So what VM images are available today, especially regarding *BSD, Solaris, MacOS? Who would be maintaining them, like, adding OS updates, installing the tools needed to build OpenVPN (on a fresh Solaris system, you can't build *anything*, for example)...
Fire up a linux VM is totally trivial :-)
[..]
Quoted 2 lines
Summary: very little maintenance is required for Vagrant. It is not like buildbot, where we actually have to build the VMs from scratch.
This sounds great but I have a hard time actually believing it...
gert
Signature · 4 lines
USENET is *not* the non-clickable part of WWW!
//www.muc.de/~gert/
Gert Doering - Munich, Germany [email protected]...
fax: +49-89-35655025 [email protected]...as for Travis-CI builds, there's such possibility already
1) register at github.com
2) add your own repo to travis-ci.org
3) voila, you can attack Pentagon from travis-ci cloud
the way of "add some attacking code to openvpn codebase" seems to be much more complicated
2016-06-07 13:20 GMT+05:00 Gert Doering <[email protected]...>:
Quoted 85 lines
Hi,
Quoted 4 lines · Samuli Seppänen
Quoted 4 lines
we can't open this to the world, as the
t_clienttests need sudo privileges, so anyone who can push a patch to a testing tree can run arbitrary commands on the buildslaves ("just build whatever you want into something called 'openvpn' and run that with sudo fromt_client")-
Quoted 3 lines
Quoted 1 line
so, (semi-)trusted developers only.
This is not entirely true, because the build steps are hardcoded.
Trivial :-) - just add a patch that will
cp
evilscript.shsrc/openvpn/openvpnat the end of the build phase.
Then, run "make check", and
t_client.shwill happily executesudo src/openvpn/openvpn $options
which now runs "
evilscript.sh" with full root access...Quoted 3 lines
However, I would definitely not open this to the world, because there is plenty of room for misuse, and Buildbot might have security issues which could be exploited.
Ideed :-)
[..]
Quoted 7 lines
It seems that a summary of how Vagrant operates is in order here.
Vagrant uses pre-built images as a starting point. These images do not (and should not) be built by OpenVPN developers. The only things _we_ have to maintain are the Vagrant files, which are basically recipies for configuring the base boxes into an OpenVPN test VMs.
Quoted 7 lines
So, when a developer wants to run the integration tests this is what happens:
- Vagrant fetches the pre-built VM images from a remote source
- The image is launched into Virtualbox (or other virtualization system)
- Vagrant runs the initialization scripts in the Vagrantfile
- The system is ready to use and stored for future useSo what VM images are available today, especially regarding *BSD, Solaris, MacOS? Who would be maintaining them, like, adding OS updates, installing the tools needed to build OpenVPN (on a fresh Solaris system, you can't build *anything*, for example)...
Fire up a linux VM is totally trivial :-)
[..]
Quoted 2 lines
Summary: very little maintenance is required for Vagrant. It is not like buildbot, where we actually have to build the VMs from scratch.
This sounds great but I have a hard time actually believing it...
gert
--
USENET is *not* the non-clickable part of WWW!//www.muc.de/~gert/
Gert Doering - Munich, Germany
[email protected]...
fax: +49-89-35655025
[email protected]...------------------------------------------------------------------------------
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are
consuming the most bandwidth. Provides multi-vendor support for NetFlow, J-Flow, sFlow and other flows. Make informed decisions using capacity planning reports. https://ad.doubleclick.net/ddm/clk/305295220;132659582;e
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Hi,
Quoted 8 lines · ???????? ??????????????
as for Travis-CI builds, there's such possibility already
1) register at github.com
2) add your own repo to travis-ci.org
3) voila, you can attack Pentagon from travis-ci cloudthe way of "add some attacking code to openvpn codebase" seems to be much more complicated
This is not about "attacking other people" - it's about compromising the integrity of my buildslaves (which could then be used to do anything, like "host malware downloads" etc).
Is this a very likely scenario, and likely to go undetected? No :-) - but still, I'm not opening these systems to Joe Random and his friends.
gert
Signature · 4 lines
USENET is *not* the non-clickable part of WWW!
//www.muc.de/~gert/
Gert Doering - Munich, Germany [email protected]...
fax: +49-89-35655025 [email protected]...Quoted 16 lines
Quoted 8 lines
It seems that a summary of how Vagrant operates is in order here.
Vagrant uses pre-built images as a starting point. These images do not (and should not) be built by OpenVPN developers. The only things _we_ have to maintain are the Vagrant files, which are basically recipies for configuring the base boxes into an OpenVPN test VMs.
Quoted 7 lines
So, when a developer wants to run the integration tests this is what happens:
- Vagrant fetches the pre-built VM images from a remote source
- The image is launched into Virtualbox (or other virtualization system)
- Vagrant runs the initialization scripts in the Vagrantfile
- The system is ready to use and stored for future use
Quoted 4 lines
So what VM images are available today, especially regarding *BSD, Solaris, MacOS? Who would be maintaining them, like, adding OS updates, installing the tools needed to build OpenVPN (on a fresh Solaris system, you can't build *anything*, for example)...
Take a look here:
<https://atlas.hashicorp.com/boxes/search?vagrantcloud=1>
All the *BSD and Solaris seem to be covered. There are probably other sources of Vagrant boxes, but that is the usual place to look for.
Quoted 4 lines
Quoted 2 lines
Summary: very little maintenance is required for Vagrant. It is not like buildbot, where we actually have to build the VMs from scratch.
This sounds great but I have a hard time actually believing it...
This is because you are a non-believer. And that in turn is because you have not tried Vagrant :). But seriously, Vagrant is an excellent tool for managing testing environments, and in our case for reproducing OS-specific problems. If you choose to not use the default (Virtualbox) then you will have more trouble, but it's still doable.
Signature · 5 lines
Samuli Seppänen Community Manager OpenVPN Technologies, Inc irc freenode net: mattock
Hi,
Quoted 6 lines · Selva Nair
The joy was short-lived. mingw cross-compile from release/2.3 tarball is also broken:
error: cannot find input file: `vendor/Makefile.in'
Thankfully, cherry-pick
41ab12f06253cfixes it.
I have cherry-picked 41ab12f06253c this for the time being into release/2.3:
commit b76acd72cd8ea295b925e058336440ff819ca8f4
Author: David Sommerseth <[email protected]...> Date: Fri Jun 3 18:56:20 2016 +0200
Another fix related to unit test framework
Signed-off-by: David Sommerseth <[email protected]...>
Acked-by: Gert Doering <[email protected]...>
Message-Id: <[email protected]...>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11778
(cherry picked from commit 41ab12f06253cadc34fc47da865178de3db0bbdc)Quoted 2 lines
IMO, the unit testing patches shouldn't have been merged into the release branch
I have discussed this with David (after the fact, meh) and we couldn't really come to an conclusion on this - but since you and Jens now both argue that way (and especially Jens as the original author should have some authority here :-) ) I think we'll revert the four relevant patches again. Looks silly, but keeping stuff in a tree where we did not want it to go in the first place, and then spending time maintaining it there looks even more silly to me...
gert
Signature · 4 lines
USENET is *not* the non-clickable part of WWW!
//www.muc.de/~gert/
Gert Doering - Munich, Germany [email protected]...
fax: +49-89-35655025 [email protected]...Quoted 5 lines
Quoted 4 lines
IMO, the unit testing patches shouldn't have been merged into the release branch
Quoted 7 lines
I have discussed this with David (after the fact, meh) and we couldn't really come to an conclusion on this - but since you and Jens now both argue that way (and especially Jens as the original author should have some authority here :-) ) I think we'll revert the four relevant patches again. Looks silly, but keeping stuff in a tree where we did not want it to go in the first place, and then spending time maintaining it there looks even more silly to me…
I agree to remove them from the next stable, but honestly think we really should go the last step and make them work in master.
Is anything besides mingw still an issue with the unit testing patchset?
Jens
A message that is not in the archive (sent off-list or not yet received): [email protected]...
Hi,
Quoted 4 lines · Jens Neuhalfen
I agree to remove them from the next stable, but honestly think we really should go the last step and make them work in master.
Is anything besides mingw still an issue with the unit testing patchset?
As far as I can see, with David's latest tweaks, all the buildbots are happy - all green, no more test fails (well, there was one, but that was due to two different tests running on the same machine in the same network namespace, so two openvpn clients trampled on each other's routing table), and building windows installers (=mingw) succeeded as well.
So, I think we've at least fixed the fallout - and since Steffann reviewed the patches fairly thoroughly, I assume that they actually work :-) (have not had time to look into that and play with cmocka myself)
gert
Signature · 4 lines
USENET is *not* the non-clickable part of WWW!
//www.muc.de/~gert/
Gert Doering - Munich, Germany [email protected]...
fax: +49-89-35655025 [email protected]...Quoted 30 lines · Selva Nair
On Fri, Jun 3, 2016 at 2:37 PM, Selva Nair <[email protected]... <mailto:[email protected]...>> wrote:
On Fri, Jun 3, 2016 at 2:14 PM, Gert Doering <[email protected]... <mailto:[email protected]...>> wrote: On Fri, Jun 03, 2016 at 07:49:23PM +0200, David Sommerseth wrote: > From: David Sommerseth <[email protected]... <mailto:[email protected]...>> > > Continuing to fix breakage caused by commit 40cb4cfc5d011102. [..] Not having actually tested this, looking at it seems to properly quote the bits that caused buildbot explosion and "make dist" fails (also as discussed between David and Samuli on IRC). So - ACK! Please apply and let's make the buildbots all green again :-) Don't know about bots, at least one human cross-compiling for windows is happy now..
Quoted 6 lines
The joy was short-lived. mingw cross-compile from release/2.3 tarball is also broken:
error: cannot find input file: `vendor/Makefile.in'
Thankfully, cherry-pick
41ab12f06253cfixes it.
Whoops! I completely forgot to include this fix into the release/2.3 branch.
Quoted 2 lines
IMO, the unit testing patches shouldn't have been merged into the release branch
I did think consider this, but ended up applying them to release/2.3 as well. My thought is that once we start having more unit tests included, we should be able to at least add some of those tests to the release/2.3 branch too. There are code which is refactored so much it won't be easy to do, but there are also plenty of code which haven't changed that dramatically.
I see these first patches from Jens as framework (or infrastructure) patches, it doesn't mean we commit ourselves to adding unit-testing to release/2.3, but it gives the possibility to do so. Our 2.3 releases will continue to live in parallel with the future 2.4 release for quite some time.
Signature · 3 lines
kind regards, David Sommerseth
Hi,
Quoted 11 lines · David Sommerseth
Quoted 3 lines
IMO, the unit testing patches shouldn't have been merged into the release branch
I did think consider this, but ended up applying them to release/2.3 as well. My thought is that once we start having more unit tests included, we should be able to at least add some of those tests to the release/2.3 branch too. There are code which is refactored so much it won't be easy to do, but there are also plenty of code which haven't changed that dramatically.
Quoted 5 lines
I see these first patches from Jens as framework (or infrastructure) patches, it doesn't mean we commit ourselves to adding unit-testing to release/2.3, but it gives the possibility to do so. Our 2.3 releases will continue to live in parallel with the future 2.4 release for quite some time.
I'm of the view that "unit tests" aid development but not much useful for maintenance. So why add unit tests to a stable code-base that should only get bug fixes (i.e., fixes for bugs that show up in real use).
On top of that, adding unit tests into an existing code like openvpn will involve a lot of refactoring. The ROI on backporting any such tests to 2.3 does not look worth the effort.
Selva
Quoted 24 lines · Selva Nair
Hi,
Quoted 2 lines · On Mon, Jun 6, 2016 at 3:05 PM, David Sommerseth <[email protected]...…
IMO, the unit testing patches shouldn't have been merged into the release branch
I did think consider this, but ended up applying them to release/2.3 as well. My thought is that once we start having more unit tests included, we should be able to at least add some of those tests to the release/2.3 branch too. There are code which is refactored so much it won't be easy to do, but there are also plenty of code which haven't changed that dramatically.
I see these first patches from Jens as framework (or infrastructure) patches, it doesn't mean we commit ourselves to adding unit-testing to release/2.3, but it gives the possibility to do so. Our 2.3 releases will continue to live in parallel with the future 2.4 release for quite some time.
Quoted 3 lines
I'm of the view that "unit tests" aid development but not much useful for maintenance. So why add unit tests to a stable code-base that should only get bug fixes (i.e., fixes for bugs that show up in real use).
True. But sometimes we will need to adjust the backport too. In those cases then we loose the unit-test result from the master branch.
Quoted 3 lines
On top of that, adding unit tests into an existing code like openvpn will involve a lot of refactoring. The ROI on backporting any such tests to 2.3 does not look worth the effort.
Yes, lots of the code may need to be refactored. But I am not currently convinced everything will need refactoring.
I am especially having the SSL abstraction layer in my mind. That did already go through a massive amount of refactoring to make it possible to swap between OpenSSL and mbedTLS/PolarSSL. Those code paths I see great value of having tested with unit tests, also in 2.3. Without having dug too much into the buffer.[ch] code lately - so my memory may be corrupted, but I think that is also code which can be adopted without too much refactoring - and these code paths are also quite crucial. But yes, the unit test coverage will not be as massive as it hopefully will develop to in master and 2.4.
From my point of view, having good unit tests on the security/privacy related code paths in 2.3 would be a good goal.
Signature · 3 lines
kind regards, David Sommerseth
Hi,
Quoted 11 lines · David Sommerseth
Quoted 3 lines
On top of that, adding unit tests into an existing code like openvpn will involve a lot of refactoring. The ROI on backporting any such tests to 2.3 does not look worth the effort.
Yes, lots of the code may need to be refactored. But I am not currently convinced everything will need refactoring.
I am especially having the SSL abstraction layer in my mind. That did already go through a massive amount of refactoring to make it possible to swap between OpenSSL and mbedTLS/PolarSSL. Those code paths I see great value of having tested with unit tests, also in 2.3.
This all sounds nice on paper, but please look more closely at what we are supposed to *do* in the release/2.3 branch
- add bug fixes
- add security relevant changes, if they are small and localized
- aim for maximum stability
there is no "refactoring" here (the SSL refactoring happened in git master before 2.3 was forked).
Unit tests in git master are good and welcome, and if we discover bugs there, we will backport the bug fixes to 2.3 (or at least look carefully if 2.3 is affected by the same bugs at all).
Given that we do not have plenty of developer time around, we should focus *development* time on git master / 2.4 - and that includes adding testing frameworks.
Ceterum censo: please reconsider whether having that stuff in 2.3 is beneficial, or just eats up highly valuable developer time without bringing enough benefits to compensate.
[..]
Quoted 6 lines
Without
having dug too much into the buffer.[ch] code lately - so my memory may be corrupted, but I think that is also code which can be adopted without too much refactoring - and these code paths are also quite crucial. But yes, the unit test coverage will not be as massive as it hopefully will develop to in master and 2.4.
"without too much refactoring" is already "way too much refactoring" - the last times we thought we'd "clean up some stuff" we introduced memory leaks and crashes that took us months to find and fix - look at ad49fb5a74b2ac16298ee8383d514a950ece1097, for example.
Since I'm usually the one who gets to debug such fallout, I'm absolutely opposed to doing any sort of "this should not change anything" or "just a bit of cleanup" changes in release/2.3. Bug-fixes, and long-term compatibility changes (if localized).
gert
Signature · 4 lines
USENET is *not* the non-clickable part of WWW!
//www.muc.de/~gert/
Gert Doering - Munich, Germany [email protected]...
fax: +49-89-35655025 [email protected]...Quoted 3 lines
Given that we do not have plenty of developer time around, we should focus *development* time on git master / 2.4 - and that includes adding testing frameworks.
+1. We should focus on doing what it takes to (finally) get 2.4 out of the door. Fairly soon afterwards we can stop worrying about 2.3 and the rather large incompatibilities between release/2.3 and master branch.
Post-2.4 we should start releasing major releases more often and with fewer features, so that the stable branch is not _that_ old when the next major release is made. That way the "release" and "master" branches do not diverge too much, and there is less need for cherry-picking patches and separately testing patching on the "release" branch. An added benefit is that more people will be using more recent code.
Signature · 5 lines
Samuli Seppänen Community Manager OpenVPN Technologies, Inc irc freenode net: mattock
Quoted 8 lines · Samuli Seppänen
Quoted 1 line
Given that we do not have plenty of developer time around, we should
focus
Quoted 2 lines
*development* time on git master / 2.4 - and that includes adding testing frameworks.
+1. We should focus on doing what it takes to (finally) get 2.4 out of the door. Fairly soon afterwards we can stop worrying about 2.3 and the rather large incompatibilities between release/2.3 and master branch.
To that I'll add that, with limited developer time, we are better off concentrating on quality activities like code-review, functional and system testing as opposed to unit testing..
Gert has already said most of the things I was holding back on saying about unit testing. In any case, unit testing in 2.3 is not something we should be worrying about.
Selva
Quoted 18 lines · Gert Doering
Hi,
Quoted 11 lines · David Sommerseth
Quoted 3 lines
On top of that, adding unit tests into an existing code like openvpn will involve a lot of refactoring. The ROI on backporting any such tests to 2.3 does not look worth the effort.
Yes, lots of the code may need to be refactored. But I am not currently convinced everything will need refactoring.
I am especially having the SSL abstraction layer in my mind. That did already go through a massive amount of refactoring to make it possible to swap between OpenSSL and mbedTLS/PolarSSL. Those code paths I see great value of having tested with unit tests, also in 2.3.
This all sounds nice on paper, but please look more closely at what we are supposed to *do* in the release/2.3 branch
Quoted 6 lines
- add bug fixes
- add security relevant changes, if they are small and localized
- aim for maximum stabilitythere is no "refactoring" here (the SSL refactoring happened in git master before 2.3 was forked).
Exactly. The unit-test code is not intrusive code at all, it lives perfectly fine on its own in parallel. And you're right about the SSL refactoring, that was my bad memory.
Quoted 11 lines
Unit tests in git master are good and welcome, and if we discover bugs there, we will backport the bug fixes to 2.3 (or at least look carefully if 2.3 is affected by the same bugs at all).
Given that we do not have plenty of developer time around, we should focus *development* time on git master / 2.4 - and that includes adding testing frameworks.
Ceterum censo: please reconsider whether having that stuff in 2.3 is beneficial, or just eats up highly valuable developer time without bringing enough benefits to compensate.
It only eats up developer resources if we put energy into it. The current state of the unit-test framework is done in a very modular way so I think what we end up maintaining in 2.3 will not be as bad as you fear.
As the 2.3 code base will fall behind 2.4 and git master over time, it only gets more important to have some unit-tests on the crucial code paths. I do not say we will put enormous resources backporting all unit tests to 2.3, but those which can be backported with reasonable efforts should be considered to be backported. This kind of evaluation of what to backport or not is not that much different to what enterprise Linux distributions already does when providing 10+ years support across some thousand packages (where rebasing to newer versions are mostly avoided).
I have personal interest in the unit-test framework, as after the last hackathon I started a similar approach for the plug-ins API. For the plugin.c there needs to be done some "untangling", but that should not cost much review efforts, as it is mostly to re-arrange the order include files are loaded - in the worst case some functions are moved from one file to another. That isn't overly complicated. But I'd presume that once that job is done, the rest of the code will be even easier to instrument with unit tests.
I do *not* say that 2.3 will carry all unit-tests from 2.4 or master. But I do not see the efforts needed to instrument 2.3 in addition as complicated as you fear. And especially now when the core unit-test framework is cleaned up and works fine in 2.3 too.
My point is: Leave the unit tests in 2.3, keeping it there doesn't cause any big maintenance issues - it is well confined, it not is enabled by default and only enabled if the person(s) building openvpn takes extra explicit steps. And this enables us to make use of it in 2.3 when the unit-test backport efforts are reasonable.
Quoted 4 lines
Since I'm usually the one who gets to debug such fallout, I'm absolutely opposed to doing any sort of "this should not change anything" or "just a bit of cleanup" changes in release/2.3. Bug-fixes, and long-term compatibility changes (if localized).
This only tells me that we are doing something very wrong. Then we need to find better ways to catch these issues at an earlier stage, so they can be found before it hits our git official repos. Vagrant and having documented ways how to run tests in Travis (or similar CIs) on your own comes to mind.
Opposing efforts to improve our current situation due to our own faulty processes are just wrong.
Signature · 3 lines
kind regards, David Sommerseth
From: David Sommerseth <[email protected]...>
Your patch has been applied to the master branch.
commit 41ab12f06253cadc34fc47da865178de3db0bbdc
Author: David Sommerseth Date: Fri Jun 3 18:56:20 2016 +0200
Another fix related to unit test framework
Signed-off-by: David Sommerseth <[email protected]...>
Acked-by: Gert Doering <[email protected]...>
Message-Id: <[email protected]...>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11778--
kind regards,
David Sommerseth