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

1.0 patch #2

Merged
merged 7 commits into from
Mar 17, 2016
Merged

1.0 patch #2

merged 7 commits into from
Mar 17, 2016

Conversation

yurrriq
Copy link
Member

@yurrriq yurrriq commented Mar 17, 2016

Add ltest as dependency under the test profile so rebar3 eunit can succeed.

Add LFE-EXPAND-EXPORTED-MACRO to the list of functions to skip in kla:filter-funcs/1. Without that the build was failing with a {redef_fun,{'LFE-EXPAND-EXPORTED-MACRO',3}} error.

This patch seems to solve the build problems wrt clj and lutil, too. 🎉

Bump the version to 0.6.1 for good measure.

Without ltest as a dependency, rebar3 eunit fails. Add it under the
test profile so it's not unnecessarily downloaded while compiling kla
as a dependency.
Add LFE-EXPAND-EXPORTED-MACRO to skips in filter-funcs/1 to avoid the
following compiler error:

    {redef_fun,{'LFE-EXPAND-EXPORTED-MACRO',3}}
@rvirding
Copy link

I also add another function by default and that is $handle_undefined_function/2. You should probably not remove this one unless you really have to as it can also be defined by the user in which case removing it would be an error. I user it for the run-time macro expansion and if we remove this then I won't be adding the function.

$handle_undefined_function/2 is called by the error handler if a call has been made to an undefined function in that module. I use it to check if there is an exported macro with that name and if so expand it otherwise just do the default thing.

@oubiwann
Copy link
Member

Thanks, guys! Nice find, Eric! I'll update the other ticket to point to here.

@yurrriq: would you mind replacing .travis.yml with the following so your PR can pass?

language: erlang
before_install:
  - wget https://s3.amazonaws.com/rebar3/rebar3
  - chmod +x rebar3
  - sudo mv rebar3 /usr/bin
  - sudo echo "#!/usr/bin/env bash" > `which rebar`
script:
  - make check
otp_release:
  - 18.2
  - 17.5
  - R16B03-1
  - R15B03

@oubiwann
Copy link
Member

@yurrriq Also, could you filter the additional function that @rvirding mentioned? $handle_undefined_function/2 ...

Thanks so much for finding this issue!

@yurrriq
Copy link
Member Author

yurrriq commented Mar 17, 2016

Yeah, I can do both.

It would be good to avoid sudo someday. I think the builds are faster or something. Can't remember...

... per Duncan's suggestion, with my own tweaks.

lfex#2 (comment)
Try to fix the failing build wrt old rebar commands.
@yurrriq
Copy link
Member Author

yurrriq commented Mar 17, 2016

Since this problem is so deeply entangled with multiple repos to which I don't have access. It's a tricky fix.. I'll do my best to make a serious of PRs (across the repos) you can pull in order.

@yurrriq
Copy link
Member Author

yurrriq commented Mar 17, 2016

The tests for this patch can't succeed until it's pulled and lutil is updated to this patched version of kla and clj is update to that patched version of lutil.. Yikes!

@yurrriq
Copy link
Member Author

yurrriq commented Mar 17, 2016

Wow this is a mess.. Basically we'll need to:

I'm getting dizzy now.. Maybe you can make more sense of this, @oubiwann...

@yurrriq
Copy link
Member Author

yurrriq commented Mar 17, 2016

Maybe we can hop on IRC or similar a little later and chat our way through this. Unfortunately I need these fixes ASAP to get some work code functioning properly again...

before_install:
- wget https://s3.amazonaws.com/rebar3/rebar3
- chmod +x rebar3
install: true
Copy link
Member Author

Choose a reason for hiding this comment

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

Without this, Travis tries to do rebar get-deps etc.

@yurrriq
Copy link
Member Author

yurrriq commented Mar 17, 2016

Let --> mean depends on.

ltest --> clj --> kla
  |       ^
  |       |
  \----> lutil

That means we'll have to update ltest similarly too, in order for the kla tests to pass.

@oubiwann
Copy link
Member

Yeah, this is the sort of think I was running into with the rebar3 conversion. It's no problem, though -- it just means doing a bunch of updates one after the other.

I'll take what you've done, branch it, do the updates, etc. I just wanted to make sure you got as much git-commit-credit as possible for the work :-)

Note, however, that I'm going to change the actual version number back to 0.6.0 (and delete the tag that's currently up on GH). I know this is an anti-pattern, but it makes things so much more simple, and I'm okay with that :-)

Be prepared for breakage, and we'll get it ironed out :-) Have the rm -rf _build rebar.lock command handy ...

@oubiwann
Copy link
Member

Note that this line:

  - sudo echo "#!/usr/bin/env bash" > `which rebar`

is the quickest way I could find of preventing rebar from running in the older versions of Erlang on Travis CI. There might be a better way, but after 10 minutes I couldn't figure out what that would be (overriding the right point in the Travis workflow didn't seem to do the trick), so I just did something quick and dirty.

Hrm, that being said, the problem target env might have been 14R, and not 15R.

Anyway, I'm going to add it back in.

@oubiwann oubiwann merged commit cd0d3f8 into lfex:master Mar 17, 2016
@yurrriq
Copy link
Member Author

yurrriq commented Mar 17, 2016

Cool, sounds good. I'm almost to the cafe so I can test etc. I'll login to IRC too in case you wanna chat.

@yurrriq
Copy link
Member Author

yurrriq commented Mar 17, 2016

The difference between using privileged or unprivileged containers on Travis is unclear to me, but install: true gets the job done too fwiw.

@yurrriq yurrriq deleted the 1.0-patch branch March 17, 2016 19:25
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

3 participants