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.

Sign up for GitHub

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

Jump to bottom

test: change time-out script to one from coreutils #3152

Merged
merged 3 commits into from
Sep 1, 2016
Merged

test: change time-out script to one from coreutils #3152

merged 3 commits into from
Sep 1, 2016

Conversation

Kubuxu
Copy link
Member

@Kubuxu Kubuxu commented Aug 30, 2016

I had a problem with finding definitive license statement of the previous
script. It was from examples in bash source tree but its licensing was a
bit unclear.

License: MIT
Signed-off-by: Jakub Sztandera kubuxu@protonmail.ch

@Kubuxu Kubuxu added the status/in-progress In progress label Aug 30, 2016
@Kubuxu Kubuxu mentioned this pull request Aug 30, 2016
Merged
@Kubuxu Kubuxu added need/review Needs a review and removed status/in-progress In progress labels Aug 30, 2016
@Kubuxu Kubuxu added this to the ipfs-0.4.3-rc4 milestone Aug 30, 2016
@whyrusleeping
Copy link
Member

Thanks @Kubuxu, This LGTM. @jbenet?

jbenet
jbenet reviewed Aug 30, 2016
View reviewed changes
test/bin/time-out

# Execute a command with a timeout

# License: LGPLv2
Copy link
Member

@jbenet jbenet Aug 30, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cant use LGPLv2

Copy link
Member Author

@Kubuxu Kubuxu Aug 30, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I know we can't link to LGPL, we can use it as a script.

Sharness is on GPLv2 and we are using it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Kubuxu we are not embedding sharness in our repo, which we cannot do. We are downloading it to develop with, which is fine with licensing. check how the Makefile does it.

Copy link
Member

@jbenet jbenet Aug 31, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I know we can't link to LGPL, we can use it as a script.

Can we embed it in our source? i don't think we can. Point me to clause where you think it's fine so i can verify? (would be nice if so)

@jbenet
Copy link
Member

jbenet commented Aug 30, 2016

It was from examples in bash source tree but its licensing was a
bit unclear.

maybe point me to it?

@Kubuxu
Copy link
Member Author

Kubuxu commented Aug 30, 2016 •
edited
Loading

The previous script: http://git.savannah.gnu.org/cgit/bash.git/tree/examples/scripts/timeout3?h=bash-4.0

EDIT: changed link

@jbenet
Copy link
Member

jbenet commented Aug 31, 2016

Fun history: http://git.savannah.gnu.org/cgit/bash.git/log/examples/scripts/timeout3?h=bash-4.3-testing

yeah idk, that's sketchy. wish licensing wasnt an issue, but we have to be super careful about this.

@jbenet
Copy link
Member

jbenet commented Aug 31, 2016

This is fun, this code is all over the place: https://github.com/search?p=1&q=Dmitry+V+Golovashkin&ref=opensearch&type=Code

@jbenet
Copy link
Member

jbenet commented Aug 31, 2016 •
edited
Loading

TL;DR: @golovashkin could you please license your timeout3 script written for bash in 2011 under the MIT License?

So to summarize, this script: http://git.savannah.gnu.org/cgit/bash.git/tree/examples/scripts/timeout3?h=bash-4.0 -- can be found all over the place (https://github.com/search?p=1&q=Dmitry+V+Golovashkin&ref=opensearch&type=Code), but always the license is unclear. People try to hand-wave around this by "giving full credit to"... and so on, which is not proper licensing, and then they bundle it in their software with other licenses (MIT, and so on). In fact, the FSF had it removed from the bash source code, probably because of unclear licensing.

It would be great for the world if Dmitry could just post this exact script and license it MIT, on github, or as a gist. @golovashkin doesn't seem to be a very active account, so i'm going to tweet at him (@golovashkin there too) to see if he could license it MIT for us. Update: i tweetted.

@whyrusleeping
Copy link
Member

@Kubuxu could we not just do: sleep 10 | COMMAND ?

jbenet
jbenet reviewed Aug 31, 2016
View reviewed changes
test/sharness/t0500-issues-and-regressions-offline.sh
@@ -8,17 +8,17 @@ test_description="Tests for various fixed issues and regressions."

test_expect_success "ipfs init with occupied input works - #2748" '
export IPFS_PATH="ipfs_path"
echo "" | time-out ipfs init &&
echo "" | time-out 10 ipfs init &&
rm -rf ipfs_path
'
test_init_ipfs

test_expect_success "ipfs cat --help succeeds with no input" '
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the text here is not right, it should be "ipfs cat --help succeeds even if stdin remains open"

correct me if i'm wrong, but afaiu (from chatting with @whyrusleeping) this is the case.

@jbenet jbenet assigned Kubuxu and unassigned jbenet Aug 31, 2016
@whyrusleeping
Copy link
Member

@Kubuxu alright, lets take the easy way out. Write a simple go program to do this.

@Kubuxu Kubuxu force-pushed the feat/test/time-out-license branch from c047082 to ab4be10 Compare August 31, 2016 17:57
@Kubuxu
Copy link
Member Author

Kubuxu commented Aug 31, 2016 •
edited
Loading

I changed the script to self written go program.
I also addressed the test name comment.

Let's wait for the tests to run.
LGTY?

@Kubuxu Kubuxu force-pushed the feat/test/time-out-license branch from ab4be10 to f025327 Compare August 31, 2016 18:19
@Kubuxu
Copy link
Member Author

Kubuxu commented Aug 31, 2016

Fixed Makefile.

@Kubuxu
Copy link
Member Author

Kubuxu commented Aug 31, 2016

Oh, the 0.4.3-rcX is at Go 1.6.

@Kubuxu
Copy link
Member Author

Kubuxu commented Aug 31, 2016

I created: #3163 which should resolve the problem with Travis and Circle if we want update the RC to Go 1.7. I think that @whyrusleeping was planning that.

@Kubuxu
test: introduce go-timeout
7639a2f 
License: MIT
Signed-off-by: Jakub Sztandera <kubuxu@protonmail.ch>
@Kubuxu
test: add go-timeotu to test build system
f012b5a 
License: MIT
Signed-off-by: Jakub Sztandera <kubuxu@protonmail.ch>
@Kubuxu
test: remove time-out script, use go-timeout instead
f59751d 
License: MIT
Signed-off-by: Jakub Sztandera <kubuxu@protonmail.ch>
@Kubuxu Kubuxu force-pushed the feat/test/time-out-license branch from f025327 to f59751d Compare September 1, 2016 13:59
@Kubuxu
Copy link
Member Author

Kubuxu commented Sep 1, 2016

Rebased.

@whyrusleeping
Copy link
Member

LGTM, thanks @Kubuxu!

@whyrusleeping whyrusleeping merged commit 0e5e6cf into version/0.4.3-rc4 Sep 1, 2016
@whyrusleeping whyrusleeping deleted the feat/test/time-out-license branch September 1, 2016 14:43
@golovashkin
Copy link

Folks, you are welcomed to use the script in any way or shape or form, however you like it.
You are also very welcomed to modify it, add the MIT license to it, and resubmit it back to Bash / Chet Ramey.

My SAS email is no longer valid, please use
golovashkin at gmail.com
(feel free to update the email in the script as well.)

Thank you!!
Best Regards,
DG

@jbenet
Copy link
Member

jbenet commented Sep 2, 2016

Thanks very much @golovashkin -- released it that way at https://github.com/jbenet/bash-timeout3 for posterity.

@Kubuxu Kubuxu removed their assignment Sep 12, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
need/review Needs a review
Projects
None yet
Milestone
ipfs-0.4.3-rc4
Development

Successfully merging this pull request may close these issues.
4 participants
@Kubuxu @whyrusleeping @jbenet @golovashkin