Resolving several travis-ci issues:
PATCH 1/1 Resolving several travis-ci issues:
* moving LD_LIBRARY_PATH as far as possible (otherwise "wget" picks it, which is not desirable)
* split LD_LIBRARY_PATH into LD_LIBRARY_PATH/DYLD_LIBRARY_PATH depending on operating system
* removed MBEDTLS_VERSION, OPENSSL_VERSION (the last depended on nonexistent OPENSSL_VERION variable)
* removed "apt: true" from caching, it never worked actually
this patch should be applied after https://www.mail-archive.com/[email protected]/msg13843.html
--- .travis.yml | 4 ++-- .travis/build-deps.sh | 2 -- 2 files changed, 2 insertions(+), 4 deletions(-)
.travis.yml +2 -2
@@ -18,7 +18,6 @@ env: - OPENSSL_VERSION="1.0.1t" - OPENSSL_CFLAGS="-I${PREFIX}/include" - OPENSSL_LIBS="-L${PREFIX}/lib -lssl -lcrypto" - - LD_LIBRARY_PATH="${PREFIX}/lib:${LD_LIBRARY_PATH}" matrix: include:
@@ -63,7 +62,6 @@ addons: - linux-libc-dev cache: - apt: true ccache: true directories: - download-cache
@@ -77,6 +75,8 @@ install: - .travis/build-deps.sh > build-deps.log 2>&1 || (cat build-deps.log && exit 1) script: + - if [ "${TRAVIS_OS_NAME}" = "linux" ]; then export LD_LIBRARY_PATH="${PREFIX}/lib:${LD_LIBRARY_PATH}"; fi + - if [ "${TRAVIS_OS_NAME}" = "osx" ]; then export DYLD_LIBRARY_PATH="${PREFIX}/lib:${LD_LIBRARY_PATH}"; fi - autoreconf -vi - ./configure --with-crypto-library="${SSLLIB}" ${EXTRA_CONFIG} || (cat config.log && exit 1) - make -j$JOBS
.travis/build-deps.sh +0 -2
@@ -2,8 +2,6 @@ set -eux # Set defaults -MBEDTLS_VERSION="${MBEDTLS_VERSION:-2.2.1}" -OPENSSL_VERSION="${OPENSSL_VERION:-1.0.2h}" PREFIX="${PREFIX:-${HOME}/opt}" download_mbedtls () {
Signature · 1 line
2.5.5
can we apply it ? seems nobody minds
2017-01-10 13:04 GMT+05:00 Ilya Shipitsin <[email protected]...>:
Quoted 65 lines
* moving
LD_LIBRARY_PATHas far as possible (otherwise "wget" picks it, which is not desirable)
* splitLD_LIBRARY_PATHinto LD_LIBRARY_PATH/DYLD_LIBRARY_PATH depending on operating system
* removedMBEDTLS_VERSION,OPENSSL_VERSION(the last depended on nonexistentOPENSSL_VERIONvariable)
* removed "apt: true" from caching, it never worked actuallythis patch should be applied after https://www.mail-archive.com/
[email protected]/msg13843.html--- .travis.yml | 4 ++-- .travis/build-deps.sh | 2 -- 2 files changed, 2 insertions(+), 4 deletions(-).travis.yml +1
-2@@ -18,7 +18,6 @@ env: - OPENSSL_VERSION="1.0.1t" - OPENSSL_CFLAGS="-I${PREFIX}/include" - OPENSSL_LIBS="-L${PREFIX}/lib -lssl -lcrypto" - - LD_LIBRARY_PATH="${PREFIX}/lib:${LD_LIBRARY_PATH}" matrix: include:@@ -63,7 +62,6 @@ addons: - linux-libc-dev cache: - apt: true ccache: true directories: - download-cache@@ -77,6 +75,8 @@ install: - .travis/build-deps.sh > build-deps.log 2>&1 || (cat build-deps.log && exit 1) script: + - if [ "${TRAVIS_OS_NAME}" = "linux" ]; then exportLD_LIBRARY_PATH="${PREFIX}/lib:${LD_LIBRARY_PATH}"; fi
+ - if [ "${TRAVIS_OS_NAME}" = "osx" ]; then export
DYLD_LIBRARY_PATH="${PREFIX}/lib:${LD_LIBRARY_PATH}"; fi
- autoreconf -vi
- ./configure --with-crypto-library="${SSLLIB}" ${EXTRA_CONFIG} ||
(cat config.log && exit 1)
- make -j$JOBS.travis/build-deps.sh +0
-3@@ -2,8 +2,6 @@ set -eux # Set defaults -MBEDTLS_VERSION="${MBEDTLS_VERSION:-2.2.1}" -OPENSSL_VERSION="${OPENSSL_VERION:-1.0.2h}" PREFIX="${PREFIX:-${HOME}/opt}" download_mbedtls () { --2.5.5
Quoted 4 lines · Ilya Shipitsin
* moving
LD_LIBRARY_PATHas far as possible (otherwise "wget" picks it, which is not desirable)
* splitLD_LIBRARY_PATHinto LD_LIBRARY_PATH/DYLD_LIBRARY_PATH depending on operating system
* removedMBEDTLS_VERSION,OPENSSL_VERSION(the last depended on nonexistentOPENSSL_VERIONvariable)
* removed "apt: true" from caching, it never worked actually
Sorry about this lingering ... To the changes itself, I don't have any real possibility to evaluate these changes. From the subject and commit message, I do not understand *why* this is needed. You just describe what I can see in the patch itself.
Having an improved commit message actually describing *why* these changes are needed - that helps motivating us to apply it. We don't like to apply things "just because it fixes things". What is this "it" and why is /this/ the right way to do it?
Basically a patch with commit message consists of three parts: Problem description, Why and How. The patch diff is the How. These two others belong to the commit message and often also worthy a comment line or two in modified files as well - especially if it isn't clear why.
I would strongly recommend you to read this blog post [1] on writing commit messages. It is a really good written post on this topic.
[1] <http://chris.beams.io/posts/git-commit/>
Quoted 1 line
this patch should be applied after https://www.mail-archive.com/[email protected]/msg13843.html
That approach got rejected and we went for this [2] approach instead, will that change anything in regards to the changes in this patch?
[2]
<https://www.mail-archive.com/[email protected]/msg13911.html>
Signature · 4 lines
kind regards, David Sommerseth OpenVPN Technologies, Inc
2017-01-26 0:21 GMT+05:00 David Sommerseth <
[email protected]...>:
Quoted 24 lines · Ilya Shipitsin
Quoted 1 line
* moving
LD_LIBRARY_PATHas far as possible (otherwise "wget" picks it,which is not desirable)
Quoted 1 line
* split
LD_LIBRARY_PATHinto LD_LIBRARY_PATH/DYLD_LIBRARY_PATHdepending on operating system
Quoted 1 line
* removed
MBEDTLS_VERSION,OPENSSL_VERSION(the last depended onnonexistent
OPENSSL_VERIONvariable)Quoted 1 line
* removed "apt: true" from caching, it never worked actually
Sorry about this lingering ... To the changes itself, I don't have any real possibility to evaluate these changes. From the subject and commit message, I do not understand *why* this is needed. You just describe what I can see in the patch itself.
Having an improved commit message actually describing *why* these changes are needed - that helps motivating us to apply it. We don't like to apply things "just because it fixes things". What is this "it" and why is /this/ the right way to do it?
Basically a patch with commit message consists of three parts: Problem description, Why and How. The patch diff is the How. These two others belong to the commit message and often also worthy a comment line or two in modified files as well - especially if it isn't clear why.
Quoted 5 lines
I would strongly recommend you to read this blog post [1] on writing commit messages. It is a really good written post on this topic.
thank you for the link.
I added more detailed commit description based on that article.
may I ask you something in turn ?
I cannot read other people thoughts, if there's something wrong with my patch, there's no other known way, but your reply.
since, you are keeping silence, I've no idea whether is it wrong or not. It would be really nice if you will tell "that and that is wrong". I'm sure, there are sites about git, where it is written.
Quoted 20 lines
Quoted 2 lines
this patch should be applied after https://www.mail-archive.com/
[email protected]/msg13843.html
That approach got rejected and we went for this [2] approach instead, will that change anything in regards to the changes in this patch?
[2]
<https://www.mail-archive.com/[email protected]...
sourceforge.net/msg13911.html>--
kind regards,David Sommerseth
OpenVPN Technologies, Inc
Quoted 18 lines · Илья Шипицин
Basically a patch with commit message consists of three parts: Problem description, Why and How. The patch diff is the How. These two others belong to the commit message and often also worthy a comment line or two in modified files as well - especially if it isn't clear why.
I would strongly recommend you to read this blog post [1] on writing commit messages. It is a really good written post on this topic.
[1] <http://chris.beams.io/posts/git-commit/
<http://chris.beams.io/posts/git-commit/>>thank you for the link.
I added more detailed commit description based on that article.
Quoted 7 lines
may I ask you something in turn ?
I cannot read other people thoughts, if there's something wrong with my patch, there's no other known way, but your reply.since, you are keeping silence, I've no idea whether is it wrong or not. It would be really nice if you will tell "that and that is wrong". I'm sure, there are sites about git, where it is written.
Fair point, and this is a very common issue in most open source projects. We are not necessarily ignoring you just for the fun of it. But we do look through a lot of patches, and need to prioritise them. For example: Patches which fixes real issues in released OpenVPN versions naturally gets a higher priority than a patch fixing a man page.
We who review patches might also misunderstand the importance of them, which is why the commit message is so important. There you give the "selling point" to why we should spend time soonish on doing a proper review and ACK/NAK it.
Also, if you have outstanding patches ... it will also help getting some attention if you join the #openvpn-devel channel and ask a bit around. Further, joining the developers meeting can help too; anyone can even add items to the discussion list for those meetings. And lastly, which you did, you asked if your patch could get some attention.
So ... don't feel bad that your patches have not been processed yet. This week I managed to get some time to look at a pile of patches from Christian Hesse - he sent most of them before end of December. The reason they got my attention is that it covered my area of interest and expertise, covered issues I have identified myself and we have discussed them on the IRC channel before Christmas last year. So there was a good understanding why we needed those patches. Also, you'll even find patches from me which is lingering on the ML which have also not gained much review yet. So this happens to anyone.
In regard to "what is wrong" ... that is truly hard to give some good pointers here. But I try as much as I can to point people into the right direction. On the other side, we do expect people to also do some research and gain more knowledge when we provide some pointers. We don't like to do spoon feeding ;-)
And a git trick, which you can instantly benefit from ... when doing git commit ... add the -s argument ;-)
Signature · 4 lines
kind regards, David Sommerseth OpenVPN Technologies, Inc
Quoted 12 lines · David Sommerseth
Quoted 10 lines · Илья Шипицин
may I ask you something in turn ?
I cannot read other people thoughts, if there's something wrong with my patch, there's no other known way, but your reply.since, you are keeping silence, I've no idea whether is it wrong or not. It would be really nice if you will tell "that and that is wrong". I'm sure, there are sites about git, where it is written.
Quoted 5 lines
Fair point, and this is a very common issue in most open source projects. We are not necessarily ignoring you just for the fun of it. But we do look through a lot of patches, and need to prioritise them. For example: Patches which fixes real issues in released OpenVPN versions naturally gets a higher priority than a patch fixing a man page.
That is hardly true. I am not complaining, but just pointing out some reality that can't be helped.
We have a dozen man page or similar level improvements since 2.4.0 though at least one critical bug fix I submitted is still awaiting review. This has to be expected as it takes the right person and quality time to get certain things reviewed. Sometimes trivialities like the non-exiting inefficiency (save a microsecond once an hour?) of carrying around [[INLINE]] tag may get multiple reviews, but a bug fix may wait for ever and even lost under the pile.
It all depends what catches our fancy, personal interest, expertise and mood. No point in trying to rationalize it beyond that.
Selva
Quoted 36 lines · Selva Nair
On Fri, Jan 27, 2017 at 10:08 AM, David Sommerseth <[email protected]... <mailto:[email protected]...>> wrote:
On 27/01/17 14:56, Илья Шипицин wrote:
>>
> may I ask you something in turn ?
> I cannot read other people thoughts, if there's something wrong with my > patch, there's no other known way, but your reply.
>
> since, you are keeping silence, I've no idea whether is it wrong or not. > It would be really nice if you will tell "that and that is wrong". I'm > sure, there are sites about git, where it is written.Fair point, and this is a very common issue in most open source projects. We are not necessarily ignoring you just for the fun of it. But we do look through a lot of patches, and need to prioritise them. For example: Patches which fixes real issues in released OpenVPN versions naturally gets a higher priority than a patch fixing a man page.
That is hardly true. I am not complaining, but just pointing out some reality that can't be helped.
We have a dozen man page or similar level improvements since 2.4.0 though at least one critical bug fix I submitted is still awaiting review. This has to be expected as it takes the right person and quality time to get certain things reviewed. Sometimes trivialities like the non-exiting inefficiency (save a microsecond once an hour?) of carrying around [[INLINE]] tag may get multiple reviews, but a bug fix may wait for ever and even lost under the pile.
Quoted 2 lines
It all depends what catches our fancy, personal interest, expertise and mood. No point in trying to rationalize it beyond that.
Also a fair point, Selva. I did generalize way too much. But I do not completely agree with you.
I can't speak for the others, but I do actually scan the list for patches where I have knowledge and look for critical/important issues first. If there are patches outside my knowledge, I can definitely process the "lower hanging fruits" first if their in my field of expertise, to get them done. If I have some spare time, I often do look at stuff outside my "comfort zone" as well, but that usually can take quite a bit longer time as I need to completely understand the issue and implementation. And it's not too uncommon that some others actually manage to send the ACK/NAK before I complete my look at it.
*And* towards the end of the 2.4.0 release cycle, it was a deliberate choice to not bring in any code changes unless absolutely needed. But we did pull in several documentation patches which would not jeopardize the release. Yes, we let known bugs pass - we evaluated these known issues and patches, considered the risks and decided that several of them should go into 2.4.1, to be sure 2.4.0 would be as stable as possible. Basically better to have a few known issues than adding more code which could make things even worse. That is a hard and tough decision to take, and it often doesn't look pretty to those not involved in those discussions. This may very well also have skewed the impression of the normal working approach we try to have.
But I do understand and accept that from an outsiders point of view, seeing all reviewers as a single instance, it may very well be experienced very differently. But we're not a single unit. We're a handful persons with very different set of skills, working somewhat unsynchronized in parallel - mostly trying to prioritize the important things first, within our own scope of skills. The unified end result of that can indeed look much more like a bag filled of random patches.
The fact is that there are quite few on this list who dare to ACK/NAK patches. If more had just done that, things could flow quicker into the git trees. Luckily the list of those giving ACK/NACK is growing slightly, but there is still too much on our list of open issues to manage everything in a timely and appropriate manner.
Signature · 4 lines
kind regards, David Sommerseth OpenVPN Technologies, Inc