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

Test UTF-8 paths on AppVeyor #1449

Merged
merged 4 commits into from Oct 27, 2017
Merged

Conversation

dra27
Copy link
Member

@dra27 dra27 commented Oct 25, 2017

See #1444.

This is based on #1447, just so that the diff is unaltered; it's also dependent on #1446 and #1448

@shindere
Copy link
Contributor

shindere commented Oct 25, 2017 via email

@dra27
Copy link
Member Author

dra27 commented Oct 25, 2017

I wouldn't type it in a shell, but I generally prefer to be explicit in a script. That said, I would always (except when I forget!) append a slash in your version, to ensure that the files don't get created as flexdll if for some reason the directory doesn't exist.

@shindere
Copy link
Contributor

shindere commented Oct 25, 2017 via email

@gasche gasche added this to the consider-for-4.06.0 milestone Oct 26, 2017
@dra27
Copy link
Member Author

dra27 commented Oct 26, 2017

@gasche - if the dependencies end up merged, then this could be activated on the 4.06 branch, but it doesn't need consideration as such, I think?

The Readme.win32.adoc stuff has ended up in the wrong GPR - I'll cherry-pick that off to trunk and 4.06 as it should have been part of #1447.

@gasche
Copy link
Member

gasche commented Oct 26, 2017

Yes, I wouldn't mind merging this in 4.06 given that it only affects the CI (we can easily correct it after the release, and it's fine if it doesn't get in user tarballs as they don't invoke these scripts themselves on a non-development version).

I'm using "consider-for-4.06.0" because it has the same semantics to me or Damien as "4.06.0", and it more clearly suggests to others that things may still be left out if time runs out.

I'm not sure why the CI fails here, and it's not easy to see which changes are not in the PR anymore since #1447 was merged -- maybe a rebase on your end could help.

@dra27
Copy link
Member Author

dra27 commented Oct 26, 2017

It was failing because #1448 isn't merged. The changes to Readme.win32.adoc hadn't ended up in the wrong GPR - this GPR was (intentionally) based on #1447. I've now rebased it on to #1448 so while it still shouldn't be merged, it's just possible that AppVeyor will pass, unless I messed up the rebase!

@gasche
Copy link
Member

gasche commented Oct 26, 2017

unless I messed up the rebase!

I notice the subtle way you complain about having to split PRs, but I am not moved :-)

@dra27
Copy link
Member Author

dra27 commented Oct 26, 2017

Don't accuse me of subtlety - I prefer sledgehammers! 😄

Actually, what I referring to was rebasing on top of #1287 which included some nice tidying of the AppVeyor script... so nice, that I do appear to have cocked up the rebase of this one!

@dra27
Copy link
Member Author

dra27 commented Oct 26, 2017

Let's see how this one fares

This was set before running the testsuite, but it's not necessary (the
testsuite contains its own trickery to ensure that CAML_LD_LIBRARY_PATH
is correctly set).
All three builds on AppVeyor updated so that the build takes place and
has PREFIX set to a directory which includes 2+ bytes UTF-8 sequences
and UTF-16 surrogate characters.
@dra27
Copy link
Member Author

dra27 commented Oct 27, 2017

The prerequisites of this GPR are now met ... assuming it still passes CI!

@gasche
Copy link
Member

gasche commented Oct 27, 2017

Edit: the message below was already-outdated, keeping it for posterity:

This should be rebased now that #1448 is merged.

@damiendoligez damiendoligez merged commit c281fae into ocaml:trunk Oct 27, 2017
gasche pushed a commit that referenced this pull request Oct 27, 2017
Test UTF-8 paths on AppVeyor
(cherry picked from commit c281fae)
@gasche
Copy link
Member

gasche commented Oct 27, 2017

Cherry-picked in 4.06 : b975103.

@dra27 dra27 deleted the appveyor-torture branch July 6, 2021 13:52
EmileTrotignon pushed a commit to EmileTrotignon/ocaml that referenced this pull request Jan 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants