New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix appveyor #2137

Merged
merged 2 commits into from Jan 15, 2016

Conversation

Projects
None yet
8 participants
@dignifiedquire
Member

dignifiedquire commented Dec 29, 2015

No description provided.

@jbenet jbenet added the backlog label Dec 29, 2015

@dignifiedquire

This comment has been minimized.

Show comment
Hide comment
@dignifiedquire

dignifiedquire Dec 29, 2015

Member

No idea why circle ci is unhappy :(

Member

dignifiedquire commented Dec 29, 2015

No idea why circle ci is unhappy :(

@Kubuxu

This comment has been minimized.

Show comment
Hide comment
@Kubuxu

Kubuxu Dec 29, 2015

Member

It is happy for me 😄

Member

Kubuxu commented Dec 29, 2015

It is happy for me 😄

@whyrusleeping

This comment has been minimized.

Show comment
Hide comment
@whyrusleeping

whyrusleeping Dec 30, 2015

Member

@chriscool could we get some advice here? the sharness tests fail on windows CI because of path separator issues (at least, thats what it seems here). What would be a good way to do this?

Member

whyrusleeping commented Dec 30, 2015

@chriscool could we get some advice here? the sharness tests fail on windows CI because of path separator issues (at least, thats what it seems here). What would be a good way to do this?

@chriscool

This comment has been minimized.

Show comment
Hide comment
@chriscool

chriscool Dec 30, 2015

Contributor

As far as I can see the first error is this:

expecting success: 
    echo "Error: failed to take lock at $IPFS_PATH: permission denied" > init_fail_exp &&
    test_cmp init_fail_exp init_fail_out

> diff -u init_fail_exp init_fail_out
--- init_fail_exp   2015-12-29 21:34:41.168675500 +0000
+++ init_fail_out   2015-12-29 21:34:41.168675500 +0000
@@ -1 +1 @@
-Error: failed to take lock at /cygdrive/c/go/src/github.com/ipfs/go-ipfs/test/sharness/trash directory.t0020-init.sh/.badipfs: permission denied
+Error: mkdir /cygdrive/c/go/src/github.com/ipfs/go-ipfs/test/sharness/trash directory.t0020-init.sh/.badipfs: The system cannot find the path specified.

not ok 3 - ipfs init output looks good

As permissions on Windows are working differently than on Linux/MacOS. It could be expected that the error message would not be the same on Windows.

Maybe this could be tested on Windows and either the error message we test against could be changed when testing on Windows or if the test is not relevant it could be skipped on Windows (maybe with a UNIXPERM prerequisite or something).

I don't have access to a Windows machine here. I will try to have a look at other test that are failing though.

Contributor

chriscool commented Dec 30, 2015

As far as I can see the first error is this:

expecting success: 
    echo "Error: failed to take lock at $IPFS_PATH: permission denied" > init_fail_exp &&
    test_cmp init_fail_exp init_fail_out

> diff -u init_fail_exp init_fail_out
--- init_fail_exp   2015-12-29 21:34:41.168675500 +0000
+++ init_fail_out   2015-12-29 21:34:41.168675500 +0000
@@ -1 +1 @@
-Error: failed to take lock at /cygdrive/c/go/src/github.com/ipfs/go-ipfs/test/sharness/trash directory.t0020-init.sh/.badipfs: permission denied
+Error: mkdir /cygdrive/c/go/src/github.com/ipfs/go-ipfs/test/sharness/trash directory.t0020-init.sh/.badipfs: The system cannot find the path specified.

not ok 3 - ipfs init output looks good

As permissions on Windows are working differently than on Linux/MacOS. It could be expected that the error message would not be the same on Windows.

Maybe this could be tested on Windows and either the error message we test against could be changed when testing on Windows or if the test is not relevant it could be skipped on Windows (maybe with a UNIXPERM prerequisite or something).

I don't have access to a Windows machine here. I will try to have a look at other test that are failing though.

@diasdavid diasdavid removed the backlog label Jan 2, 2016

@whyrusleeping

This comment has been minimized.

Show comment
Hide comment
@whyrusleeping

whyrusleeping Jan 7, 2016

Member

I think we should merge this in for now. It still doesnt pass appveyor tests, but its gets a little farther and we can move forward from there

Member

whyrusleeping commented Jan 7, 2016

I think we should merge this in for now. It still doesnt pass appveyor tests, but its gets a little farther and we can move forward from there

@dignifiedquire

This comment has been minimized.

Show comment
Hide comment
@dignifiedquire

dignifiedquire Jan 7, 2016

Member

👍 for merging. At least actual tests are now failing

Member

dignifiedquire commented Jan 7, 2016

👍 for merging. At least actual tests are now failing

@GitCop

This comment has been minimized.

Show comment
Hide comment
@GitCop

GitCop Jan 14, 2016

There were the following issues with your Pull Request

  • Commit: d2157f5
    • Invalid signoff. Commit message must end with
      License: MIT
      Signed-off-by: .* <.*>
  • Commit: d586996
    • Invalid signoff. Commit message must end with
      License: MIT
      Signed-off-by: .* <.*>
  • Commit: 596fc03
    • Invalid signoff. Commit message must end with
      License: MIT
      Signed-off-by: .* <.*>

We ask for a few features in the commit message for Open Source licensing hygiene and commit message clarity.
git commit --amend can often help you quickly improve the commit message.
Guidelines and a script are available to help in the long run.
Your feedback on GitCop is welcome on this issue.


This message was auto-generated by https://gitcop.com

GitCop commented Jan 14, 2016

There were the following issues with your Pull Request

  • Commit: d2157f5
    • Invalid signoff. Commit message must end with
      License: MIT
      Signed-off-by: .* <.*>
  • Commit: d586996
    • Invalid signoff. Commit message must end with
      License: MIT
      Signed-off-by: .* <.*>
  • Commit: 596fc03
    • Invalid signoff. Commit message must end with
      License: MIT
      Signed-off-by: .* <.*>

We ask for a few features in the commit message for Open Source licensing hygiene and commit message clarity.
git commit --amend can often help you quickly improve the commit message.
Guidelines and a script are available to help in the long run.
Your feedback on GitCop is welcome on this issue.


This message was auto-generated by https://gitcop.com

@dignifiedquire

This comment has been minimized.

Show comment
Hide comment
@dignifiedquire

dignifiedquire Jan 14, 2016

Member

@whyrusleeping just rebased and squashed. Can we get this merged now?

Member

dignifiedquire commented Jan 14, 2016

@whyrusleeping just rebased and squashed. Can we get this merged now?

@jbenet

This comment has been minimized.

Show comment
Hide comment
@jbenet

jbenet Jan 15, 2016

Member

@dignifiedquire is it actually fixed? it says:

Cannot find the tests' local ipfs tool.
Please check test and ipfs tool installation.
Makefile:29: recipe for target 't0010-basic-commands.sh' failed
make[1]: *** [t0010-basic-commands.sh] Error 1
make[1]: Leaving directory '/cygdrive/c/go/src/github.com/ipfs/go-ipfs/test/sharness'
Makefile:59: recipe for target 'test_sharness_expensive' failed
make: *** [test_sharness_expensive] Error 2
Command exited with code 2
Member

jbenet commented Jan 15, 2016

@dignifiedquire is it actually fixed? it says:

Cannot find the tests' local ipfs tool.
Please check test and ipfs tool installation.
Makefile:29: recipe for target 't0010-basic-commands.sh' failed
make[1]: *** [t0010-basic-commands.sh] Error 1
make[1]: Leaving directory '/cygdrive/c/go/src/github.com/ipfs/go-ipfs/test/sharness'
Makefile:59: recipe for target 'test_sharness_expensive' failed
make: *** [test_sharness_expensive] Error 2
Command exited with code 2
@dignifiedquire

This comment has been minimized.

Show comment
Hide comment
@dignifiedquire

dignifiedquire Jan 15, 2016

Member

It was :( but it looks like something went wrong/changed after the rebase.

Member

dignifiedquire commented Jan 15, 2016

It was :( but it looks like something went wrong/changed after the rebase.

rht and others added some commits Nov 20, 2015

Add appveyor conf
License: MIT
Signed-off-by: rht <rhtbot@gmail.com>
Start fixing appveyor
License: MIT
Signed-off-by: dignifiedquire <dignifiedquire@gmail.com>
@dignifiedquire

This comment has been minimized.

Show comment
Hide comment
@dignifiedquire

dignifiedquire Jan 15, 2016

Member

@jbenet that was close, luckily I had the fixed version on my other machine. Should be better now

Member

dignifiedquire commented Jan 15, 2016

@jbenet that was close, luckily I had the fixed version on my other machine. Should be better now

@@ -23,6 +23,8 @@ import (
manet "github.com/ipfs/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-multiaddr-net"
)
var setupOpt = func(cmd *exec.Cmd) {}

This comment has been minimized.

@jbenet

jbenet Jan 15, 2016

Member

were these changes contributed to iptb proper?

@jbenet

jbenet Jan 15, 2016

Member

were these changes contributed to iptb proper?

This comment has been minimized.

@dignifiedquire

dignifiedquire Jan 15, 2016

Member

Yes, this was just pulled in from iptb

@dignifiedquire

dignifiedquire Jan 15, 2016

Member

Yes, this was just pulled in from iptb

This comment has been minimized.

@jbenet

jbenet Jan 15, 2016

Member

ah ok!

@jbenet

jbenet Jan 15, 2016

Member

ah ok!

This comment has been minimized.

@dignifiedquire

dignifiedquire Jan 15, 2016

Member

ipfs/iptb@b21f95b is the commit I think

@dignifiedquire

dignifiedquire Jan 15, 2016

Member

ipfs/iptb@b21f95b is the commit I think

@jbenet

This comment has been minimized.

Show comment
Hide comment
@jbenet

jbenet Jan 15, 2016

Member

Ok great! this LGTM. down to merge it before fixing the bugs. But before-- the iptb stuff should be updated in the iptb repo (https://github.com/whyrusleeping/iptb) and then update the dep here.

Member

jbenet commented Jan 15, 2016

Ok great! this LGTM. down to merge it before fixing the bugs. But before-- the iptb stuff should be updated in the iptb repo (https://github.com/whyrusleeping/iptb) and then update the dep here.

jbenet added a commit that referenced this pull request Jan 15, 2016

@jbenet jbenet merged commit 2d9f6a8 into ipfs:master Jan 15, 2016

2 of 3 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@dignifiedquire dignifiedquire deleted the dignifiedquire:app branch Jan 15, 2016

@jbenet

This comment has been minimized.

Show comment
Hide comment
@jbenet

jbenet Jan 15, 2016

Member

Thanks @dignifiedquire, @whyrusleeping, @chriscool, @Kubuxu, and whoever else was involved -- it's a big deal to get this working :)

Member

jbenet commented Jan 15, 2016

Thanks @dignifiedquire, @whyrusleeping, @chriscool, @Kubuxu, and whoever else was involved -- it's a big deal to get this working :)

@dignifiedquire

This comment has been minimized.

Show comment
Hide comment
@dignifiedquire

dignifiedquire Jan 15, 2016

Member

You have mostly to thank 32C3 dark room hack sessions :D

Member

dignifiedquire commented Jan 15, 2016

You have mostly to thank 32C3 dark room hack sessions :D

@jbenet jbenet referenced this pull request Jan 15, 2016

Closed

Sprint January 11th #79

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment