Skip to contentSkip to conversation

enable "--disable-crypto" build configuration

PATCH In review Ilya Shipitsin 6 participants 10 messages Sep 17, 2016 – Sep 26, 2016

lore
Share & tools
lore

Ilya Shipitsin

author
#0 lore raw

PATCH 1/1 enable "--disable-crypto" build configuration

---
 .travis.yml | 4 ----
 1 file changed, 4 deletions(-)
.travis.yml +0 -4
index 452c48e..369db97 100644
@@ -51,10 +51,6 @@ matrix:
       os: osx
       osx_image: xcode7.3
       compiler: clang
-  allow_failures:
-    - env: SSLLIB="openssl" EXTRA_CONFIG="--disable-crypto"
-      os: linux
-      compiler: clang
   exclude:
     - compiler: gcc
Signature · 1 line
2.5.5
#1 lore raw

Hi,

Quoted 20 lines · Ilya Shipitsin
---
 .travis.yml | 4 ----
 1 file changed, 4 deletions(-)
.travis.yml +0 -5
index 452c48e..369db97 100644
@@ -51,10 +51,6 @@ matrix:
       os: osx
       osx_image: xcode7.3
       compiler: clang
-  allow_failures:
-    - env: SSLLIB="openssl" EXTRA_CONFIG="--disable-crypto"
-      os: linux
-      compiler: clang
   exclude:
     - compiler: gcc
 
--
Quoted 1 line

2.5.5

Thanks. The patch is good, so ACK.

But the commit messages should describe why this change is needed. In this case:

Previously, 'make test' failed for --disable-crypto builds. Since that is not fixed, we should no longer accept --disable-crypto builds to fail 'make test' on travis.

Could you please include such descriptions in future patches?

-Steffan

author
#2 lore raw

2016-09-17 16:17 GMT+05:00 Steffan Karger <[email protected]...>:

Quoted 28 lines

Hi,

Quoted 20 lines · Ilya Shipitsin
---
 .travis.yml | 4 ----
 1 file changed, 4 deletions(-)
.travis.yml +0 -5
index 452c48e..369db97 100644
@@ -51,10 +51,6 @@ matrix:
       os: osx
       osx_image: xcode7.3
       compiler: clang
-  allow_failures:
-    - env: SSLLIB="openssl" EXTRA_CONFIG="--disable-crypto"
-      os: linux
-      compiler: clang
   exclude:
     - compiler: gcc
 
--
Quoted 1 line

2.5.5

Thanks. The patch is good, so ACK.

Quoted 9 lines

But the commit messages should describe why this change is needed. In this case:

Previously, 'make test' failed for --disable-crypto builds. Since that is not fixed, we should no longer accept --disable-crypto builds to fail 'make test' on travis.

Could you please include such descriptions in future patches?

ok, I'm going to play with travis-ci + valgrind soon.

or, some powershell for windows testing (it's on my list)

Quoted 3 lines

-Steffan

author
#3 lore raw
In reply to Steffan Karger

can we merge it ?

2016-09-17 16:17 GMT+05:00 Steffan Karger <[email protected]...>:

Quoted 39 lines

Hi,

Quoted 20 lines · Ilya Shipitsin
---
 .travis.yml | 4 ----
 1 file changed, 4 deletions(-)
.travis.yml +0 -5
index 452c48e..369db97 100644
@@ -51,10 +51,6 @@ matrix:
       os: osx
       osx_image: xcode7.3
       compiler: clang
-  allow_failures:
-    - env: SSLLIB="openssl" EXTRA_CONFIG="--disable-crypto"
-      os: linux
-      compiler: clang
   exclude:
     - compiler: gcc
 
--
Quoted 1 line

2.5.5

Thanks. The patch is good, so ACK.

But the commit messages should describe why this change is needed. In this case:

Previously, 'make test' failed for --disable-crypto builds. Since that is not fixed, we should no longer accept --disable-crypto builds to fail 'make test' on travis.

Could you please include such descriptions in future patches?

-Steffan

#4 lore raw

Il 24/09/2016 09:32, Илья Шипицин ha scritto:

Quoted 47 lines

can we merge it ?

2016-09-17 16:17 GMT+05:00 Steffan Karger <[email protected]... <mailto:[email protected]...>>:

Hi,

On 17 September 2016 at 12:54, Ilya Shipitsin <[email protected]... <mailto:[email protected]...>> wrote:
> ---
> .travis.yml | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/.travis.yml b/.travis.yml
> index 452c48e..369db97 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -51,10 +51,6 @@ matrix:
> os: osx
> osx_image: xcode7.3
> compiler: clang
> - allow_failures:
> - - env: SSLLIB="openssl" EXTRA_CONFIG="--disable-crypto"
> - os: linux
> - compiler: clang
> exclude:
> - compiler: gcc
>
> --
> 2.5.5

Thanks. The patch is good, so ACK.

But the commit messages should describe why this change is needed. In this case:

Previously, 'make test' failed for --disable-crypto builds. Since that is not fixed, we should no longer accept --disable-crypto builds to fail 'make test' on travis.

Could you please include such descriptions in future patches?

-Steffan

Quoted 9 lines

------------------------------------------------------------------------------

_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

All the pieces (ACK, fixed commit message) seem to be present. Maybe it would be easiest to fix the commit message just before the push:

---

Enable "--disable-crypto" build configuration

Previously, 'make test' failed for --disable-crypto builds. Since that is not fixed, we should no longer accept --disable-crypto builds to fail 'make test' on travis.

Signature · 5 lines
Samuli Seppänen
Community Manager
OpenVPN Technologies, Inc

irc freenode net: mattock
author
#5 lore raw

2016-09-26 12:37 GMT+05:00 Samuli Seppänen <[email protected]...>:

Quoted 54 lines

Il 24/09/2016 09:32, Илья Шипицин ha scritto:

Quoted 52 lines

can we merge it ?

2016-09-17 16:17 GMT+05:00 Steffan Karger <[email protected]... <mailto:[email protected]...>>:

Hi,

On 17 September 2016 at 12:54, Ilya Shipitsin <[email protected]... <mailto:[email protected]...>> wrote:
> ---
> .travis.yml | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/.travis.yml b/.travis.yml
> index 452c48e..369db97 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -51,10 +51,6 @@ matrix:
> os: osx
> osx_image: xcode7.3
> compiler: clang
> - allow_failures:
> - - env: SSLLIB="openssl" EXTRA_CONFIG="--disable-crypto"
> - os: linux
> - compiler: clang
> exclude:
> - compiler: gcc
>
> --
> 2.5.5

Thanks. The patch is good, so ACK.

But the commit messages should describe why this change is needed. In this case:

Previously, 'make test' failed for --disable-crypto builds. Since that is not fixed, we should no longer accept --disable-crypto builds to fail 'make test' on travis.

Could you please include such descriptions in future patches?

-Steffan

------------------------------------------------------------
------------------

Quoted 9 lines
Quoted 6 lines

_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

All the pieces (ACK, fixed commit message) seem to be present. Maybe it would be easiest to fix the commit message just before the push:

it would be easiest, but it is not required.
as Steffan Karger wrote "Could you please include such descriptions in future patches?"

Quoted 17 lines

---

Enable "--disable-crypto" build configuration

Previously, 'make test' failed for --disable-crypto builds. Since that is not fixed, we should no longer accept --disable-crypto builds to fail 'make test' on travis.

--
Samuli Seppänen
Community Manager
OpenVPN Technologies, Inc

irc freenode net: mattock

#6 lore raw
Quoted 11 lines · Илья Шипицин

2016-09-26 12:37 GMT+05:00 Samuli Seppänen <[email protected]... <mailto:[email protected]...>>:
All the pieces (ACK, fixed commit message) seem to be present. Maybe it would be easiest to fix the commit message just before the push:

it would be easiest, but it is not required.
as Steffan Karger wrote "Could you please include such descriptions in future patches?"

Quoted 8 lines

---

Enable "--disable-crypto" build configuration

Previously, 'make test' failed for --disable-crypto builds. Since that is not fixed, we should no longer accept --disable-crypto builds to fail 'make test' on travis.

The committer (David or Gert) can probably just include this text into the commit message when applying the patch. I think that's what Samuli suggested.

Committer: if you do so, could you please fix my typo from 'not fixed' to 'now fixed'? ;-)

Thanks,
-Steffan

#7 lore raw
In reply to Илья Шипицин

Hi,

Quoted 3 lines · ???????? ??????????????

it would be easiest, but it is not required.
as Steffan Karger wrote "Could you please include such descriptions in future patches?"

Well, a proper commit message *is* required.

So, instead of us having do your work (of providing a proper commit message), and then complaining that this is taking a bit longer (because it's extra work) - what about "re-sending the patch with a proper commit message" instead?

I have a bunch of work to do, and "make me do more work and then complain that I'm slow doing it" is a very trusted way to get moved all the way DOWN on my priority list. Like, "I might think about this next May".

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]...
#8 lore raw

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Quoted 15 lines · Gert Doering

Hi,

Quoted 3 lines · ???????? ??????????????

it would be easiest, but it is not required. as Steffan Karger wrote "Could you please include such descriptions in future patches?"

Well, a proper commit message *is* required.

So, instead of us having do your work (of providing a proper commit message), and then complaining that this is taking a bit longer (because it's extra work) - what about "re-sending the patch with a proper commit message" instead?

Quoted 4 lines

I have a bunch of work to do, and "make me do more work and then complain that I'm slow doing it" is a very trusted way to get moved all the way DOWN on my priority list. Like, "I might think about this next May".

+1 ... Due to _exactly_ this reasoning, it was on my "If I have time this week"-list.

Thanks Gert for taking care of it!

- --
kind regards,

David Sommerseth
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBAgAGBQJX6XloAAoJEIbPlEyWcf3yPrwP/i8Yu4TVUZr0waGJ6Gx9g3Z9 qMUqKSfif73+3nnPbPsEd8N0+eRHjG7dgQ52+/Sahph73dIWiiLrepRAC3VJCc1r 7ZGsFYYkoYKMf2VlWHKgi8WkuaXt7xhofHLLh0GtmmOvaGU9zcqaIZpLEPpXpfYf RULw8ozDE+rirQA2UCxht1Fqn+tf/36t0X1zCkFXplkglT7oxLr6aPHbuUUUl19v 9URwwxDGFyy4De0ZalH/cosAXRwzsyjEvG2QCFoGGsLEbiNMElpT9MImPkJFjjyc ltPtZGMW3tjdAIRIyTtMhlXZvrUu0eig+2SjCmwWZMAwL0XfSguUsEk9c13opSuT xTsAoE8c2rEXN/5UvwFIiRI9qgUwp83zQDVu6QEjlaH4/PG5iRz7kSisc2FGQtwS nYN+183nW8MQVJFSaZ++VbnOnGhpH86NmgTc2BxRInDIBEjd4uQTQVXMH1UBWzmJ 7PFrEowT3vnRF6SDFaE2YzyCP2hp1DwD0la2NCeieXcaAq0+1lSmR4UpSlPeNkO+ QujuqtyoeC372P0CnLxbOZCdrvXhBcRWGs8O1PjBzHy5xeyApgbWNoqI7kkENvk9 +WRrVPq9cQngrfr1dCo4H3c6y1wRSjzhMU7VxJ5/82+u3qPXxBIvnu/tSl24dbpM anEkoVh1m3iCyJt+aJp1
=bNDi
-----END PGP SIGNATURE-----

#9 lore raw

Re: [Openvpn-devel] [PATCH applied] enable --disable-crypto build configuration for travis

In reply to Ilya Shipitsin

Your patch has been applied to the master branch

(I've amended the commit message as suggested by Steffan, fixing the "not" while at it)

commit 0c72e29c990a766e6952455d23151dabf79ed22b

Author: Ilya Shipitsin
Date:   Sat Sep 17 15:54:39 2016 +0500
     enable --disable-crypto build configuration for travis

     Acked-by: Steffan Karger <[email protected]...>
     Message-Id: <[email protected]...>
     URL: http://www.mail-archive.com/search?l=mid&[email protected]...
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering