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

use size_t instead of unsigned in a bunch of appropriate places #425

Merged
merged 1 commit into from Mar 16, 2015

Conversation

joshuawarner32
Copy link
Collaborator

This would theoretically break compatibility with apps using embedded
classpaths, on big-endian architectures - because of the size type
extension. However, we don't currently support any big-endian
architectures, so it shouldn't be a problem.

@dicej
Copy link
Member

dicej commented Mar 16, 2015

@dicej
Copy link
Member

dicej commented Mar 16, 2015

Once again, Travis tells us the build is fine when it's not. Boo. Hiss.

This would theoretically break compatibility with apps using embedded
classpaths, on big-endian architectures - because of the size type
extension.  However, we don't currently support any big-endian
architectures, so it shouldn't be a problem.
@joshuawarner32
Copy link
Collaborator Author

Good thing you look at the output; I haven't been. When running ./test/ci.sh locally, it does what we want (fails on errors), so it's either travis's fault, or it's something weird we're doing in our configuration (.travis.yml). @sgoings, could you take a look?

@dicej
Copy link
Member

dicej commented Mar 16, 2015

Actually, I'm able to reproduce this with test/ci.sh locally, too. It looks like it might be related to our use of && and || to do conditionals instead of if/then, with the result that when "make mode=debug bootimage=true test" fails, it doesn't error out of the script; it only skips the next item in the && chain.

@dicej
Copy link
Member

dicej commented Mar 16, 2015

@dicej
Copy link
Member

dicej commented Mar 16, 2015

This seems to fix it:

diff --git a/test/ci.sh b/test/ci.sh
index 4c99e1d..69c89d2 100755
--- a/test/ci.sh
+++ b/test/ci.sh
@@ -86,17 +86,22 @@ else

   make_target=test

-  (! has_flag arch) && run make ${flags} jdk-test
+  if ! has_flag arch; then
+    run make ${flags} jdk-test
+  fi
+
   run make ${flags} ${make_target}
   run make ${flags} mode=debug ${make_target}
   run make ${flags} process=interpret ${make_target}

-  (has_flag openjdk-src || ! has_flag openjdk) && \
-    run make ${flags} mode=debug bootimage=true ${make_target} && \
+  if has_flag openjdk-src || ! has_flag openjdk; then
+    run make ${flags} mode=debug bootimage=true ${make_target}
     run make ${flags} bootimage=true ${make_target}
+  fi

-  (! has_flag openjdk && ! has_flag android && ! has_flag arch) && \
+  if ! has_flag openjdk && ! has_flag android && ! has_flag arch; then
     run make ${flags} openjdk=$JAVA_HOME ${make_target}
+  fi

   run make ${flags} tails=true continuations=true heapdump=true ${make_target}
   run make ${flags} codegen-targets=all

I'll make a pull request.

dicej added a commit that referenced this pull request Mar 16, 2015
use size_t instead of unsigned in a bunch of appropriate places
@dicej dicej merged commit 7d6f80e into ReadyTalk:master Mar 16, 2015
@joshuawarner32 joshuawarner32 deleted the size_t branch March 17, 2015 13:51
joshuawarner32 added a commit to joshuawarner32/avian that referenced this pull request May 3, 2015
I'm not sure how we didn't catch this when merging ReadyTalk#425, but there you are.
dicej added a commit that referenced this pull request May 4, 2015
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.

None yet

2 participants