Skip to content
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

bump go daemon tag version #229

Closed
wants to merge 3 commits into from
Closed

bump go daemon tag version #229

wants to merge 3 commits into from

Conversation

sinkingsugar
Copy link
Contributor

@sinkingsugar sinkingsugar commented Jun 18, 2020

includes lastest changes, and latest noise

@sinkingsugar
Copy link
Contributor Author

@stefantalpalaru would you mind checking that win 32 job at least .. it's a weird failure , stops at the p2p build script...
Cheers

@sinkingsugar
Copy link
Contributor Author

@stefantalpalaru another thing sorry.. I fail to make it work actually.. passing the version didn't do anything, is it cache related ( see travis )?

@stefantalpalaru
Copy link
Contributor

The Azure cache is not invalidated automatically. You need to manually change the cache key, for both the Nim compiler and p2pd.

The Travis problem doesn't seem cache-related.

@sinkingsugar
Copy link
Contributor Author

The Azure cache is not invalidated automatically. You need to manually change the cache key, for both the Nim compiler and p2pd.

Thanks I see.

The Travis problem doesn't seem cache-related.

It's definitely printing the old p2pd help so it seems to me that is cache related too

@stefantalpalaru
Copy link
Contributor

It's definitely printing the old p2pd help so it seems to me that is cache related too

You're right. I need to port "build_p2pd.sh" from the old binary-mtime-based system to the separate timestamp file we use in "build_nim.sh". Just a moment.

@stefantalpalaru
Copy link
Contributor

Yes. Hang on while I'm debugging it.

@stefantalpalaru
Copy link
Contributor

It's a problem with Go-1.12: libp2p/go-libp2p-daemon#212

@sinkingsugar
Copy link
Contributor Author

Oh wow, didn't realize it was so convoluted. Thanks @stefantalpalaru

@sinkingsugar
Copy link
Contributor Author

I migrated my own projects to use GitHub actions and never regret about it to be honest.
Keeping track of the quirks and workarounds of all the other providers was taking too much time I don't have !

- Travis: use Go 1.14
- azure-pipelines.yml: big cleanup
- Azure: bump cache keys
- build 64-bit p2pd on 32-bit Windows
- install both Mingw-w64 architectures
@stefantalpalaru
Copy link
Contributor

stefantalpalaru commented Jun 19, 2020

I squashed all my commits and any CI failure you may see is not our fault, so this PR is ready for merging.

Important points: the latest p2pd version does not build with Go-1.12 nor does it build on 32-bit any more.

I saw a different p2pd cache key in another PR. Make sure it doesn't overwrite this one.

I migrated my own projects to use GitHub actions

What are the chances it uses the same underlying infrastructure as Azure Pipelines?

@stefantalpalaru
Copy link
Contributor

I saw this Travis error appearing sometimes on Azure. A matter of timing?

   /home/travis/gopath/src/github.com/status-im/nim-libp2p/tests/pubsub/testfloodsub.nim(42, 31): Check failed: tracker.isLeaked() == false

    tracker.isLeaked() was true

    false was false

  [FAILED] FloodSub multiple peers, no self trigger

@sinkingsugar
Copy link
Contributor Author

I saw this Travis error appearing sometimes on Azure. A matter of timing?

   /home/travis/gopath/src/github.com/status-im/nim-libp2p/tests/pubsub/testfloodsub.nim(42, 31): Check failed: tracker.isLeaked() == false

    tracker.isLeaked() was true

    false was false

  [FAILED] FloodSub multiple peers, no self trigger

yeah indeed, often on macos too cos probably it's nested virtualization...

@sinkingsugar
Copy link
Contributor Author

Thank you for azure btw @stefantalpalaru !

@sinkingsugar
Copy link
Contributor Author

It's weird tho, the timing issues I didn't have with pubsub before tho.. now this one too:
https://travis-ci.org/github/status-im/nim-libp2p/jobs/699921676#L666

@dryajov
Copy link
Contributor

dryajov commented Jun 19, 2020

Quite possible, hard to tell from the snippet, but I haven't seen it break there in a while.

@sinkingsugar
Copy link
Contributor Author

btw I'm closing this PR and move it (merging it) into #226
Since it's complementary!

@sinkingsugar
Copy link
Contributor Author

sinkingsugar commented Jun 19, 2020

Quite possible, hard to tell from the snippet, but I haven't seen it break there in a while.

@dryajov
Well it means that await switch.stop() is not enough to await all the possible closures and so it triggers leak check... possibly eventually there will be no leaks.
When I fixed those tests that was the hard work, making sure all was closed.. maybe something is sneaking out again :)

I really would try to avoid adding sleeps or so and rather have systematical knowledge that we close all.

@arnetheduck arnetheduck deleted the daemon-bump branch September 3, 2020 11:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants