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

Fix Appveyor 2.4.1 build #343

Merged
merged 1 commit into from
Aug 20, 2017
Merged

Fix Appveyor 2.4.1 build #343

merged 1 commit into from
Aug 20, 2017

Conversation

deivid-rodriguez
Copy link
Owner

@deivid-rodriguez deivid-rodriguez commented Aug 20, 2017

Description

This PR should get Appveyor 2.4.1 build green again.

Full credit to @MSP-Greg for the tip ❤️.

Requirements

Before submitting the PR make sure the following are checked:

  • Wrote good commit messages.
  • Used the same coding conventions as the rest of the project.
  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • All tests are passing.
  • The PR relates to only one subject.

@deivid-rodriguez deivid-rodriguez merged commit e45e9f0 into master Aug 20, 2017
@deivid-rodriguez deivid-rodriguez deleted the fix/windows_build branch August 20, 2017 21:51
@MSP-Greg
Copy link
Collaborator

@deivid-rodriguez

Glad that helped. When I get a minute, I'll have a look at the two trunk fails. Also, I added rb-readline back into my two builds, as it seemed that the readline ext slowed testing down.

Of course, feel free to add my trunk build as an 'allow_failures:' job...

@deivid-rodriguez
Copy link
Owner Author

Glad that helped. When I get a minute, I'll have a look at the two trunk fails

Thank you, that'd be great :)

Also, I added rb-readline back into my two builds, as it seemed that the readline ext slowed testing down.

I added rb-readline in the past because it was supposed to fix some problems, but I had to revert it because it caused others. I can do some git digging for you if interested.

Of course, feel free to add my trunk build as an 'allow_failures:' job...

Maybe you could coordinate with Appveyor to make your ruby-head ruby official :)

@MSP-Greg
Copy link
Collaborator

I can do some git digging for you if interested.

No need. Readline and 'Ruby on Windows' is messy. I just updated the appveyor-ruby page. Every version listed (2.1 forward) uses rb-readline.

I build ruby with readline 7, mostly just to see if it tests well. But, when using irb (don't use a lot), I always disabled it. That, along with the issues testing byebug, led me to add rb-readline back in, as it's included in a way that overrides the extension files.

Maybe you could coordinate with Appveyor to make your ruby-head ruby official :)

Again, messy. I never found anywhere/anyone that logged test results for MinGW ruby builds. I've been logging online results since April, and there are still tests that fail intermittently, etc. So, an automated build via appveyor is messy. Lars is working on seeing if Ruby Core will add it to their CI, but who knows.

Because of these problems, I've been building myself, and checking the results to determine whether to post the update. I've got gem authors saying what you've said, and appveyor will probably ask whether anyone is using the build for testing. An American phrase comes to mind, 'What comes first, the chicken or the egg?'...

@deivid-rodriguez
Copy link
Owner Author

Because of these problems, I've been building myself, and checking the results to determine whether to post the update. I've got gem authors saying what you've said, and appveyor will probably ask whether anyone is using the build for testing. An American phrase comes to mind, 'What comes first, the chicken or the egg?'...

That's true, I'll gladly accept a PR to add your ruby-head build, so you can add byebug to the list of gems using it :)

@MSP-Greg
Copy link
Collaborator

Thank you. This repo would be the first that I'm aware of.

A bit busy with some other things. We can't have it added to the repo as an acceptable fail, so I'll get to the two fails, then post the PR.

@deivid-rodriguez
Copy link
Owner Author

We can't? Can't we place it under allowed failures somehow?

Note that the two failures are unrelated to Windows, since they also happen in Travis CI. Help welcome with them in any case.

@MSP-Greg
Copy link
Collaborator

@deivid-rodriguez

Did some more testing last night. As with my Appveyor, Three builds 2.3, 2.4, & trunk. ByeBug doesn't start from command line in trunk. The call to Byebug.debug_load returns with a nil error in trunk, but not 2.3 & 2.4. I'm not much a c type...

@deivid-rodriguez
Copy link
Owner Author

@MSP-Greg Thanks, that's helpful. I'll open an issue to track this.

Feel free to contribute that ruby-head build in any case, that way we'll know whether the fix for this works on Windows too :)

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.

2 participants