Skip to content
This repository has been archived by the owner on May 12, 2018. It is now read-only.

Don't over-aggressively clean the code path in the presence of lib_dir directives #157

Merged
merged 2 commits into from
Nov 22, 2013

Conversation

Vagabond
Copy link
Contributor

Rebar, when it encounters a lib_dir directive, caches the current code
path, adds the libdir(s) and returns the cached copy of the path. When
rebar has finished processing that directory, it restores the cached
path. This is problematic in the below scenario:

        /(lib_dir)->G
  A -> B -> C -> D -> E
   \-> F -> D -> E

When rebar is finished processing B, it restores the code path to what
it was before it processed B, removing C, D, E and G from the code path.
This means when it comes to process F, neither D or E are in the code
path, so any header includes, rebar plugins or parse transforms will not
be in the code path. Without the lib_dir directive, rebar does no code
path cleanups, so everything works fine.

This change makes rebar only remove the explicit lib_dir code paths it
added and adds an inttest that replicates the above scenario.

I don't actually know why rebar feels the need to do this in the first place, but this code has been in rebar since 2009, pretty much unchanged:

3990f0a

This patch still ensures the lib_dirs are stripped when exiting the directory, but if the lib_dirs contain applications with deps those will not be removed from the code path. I don't know if this is good or bad.

…r directives

Rebar, when it encounters a lib_dir directive, caches the current code
path, adds the libdir(s) and returns the cached copy of the path. When
rebar has finished processing that directory, it restores the cached
path. This is problematic in the below scenario:

        /(lib_dir)->G
  A -> B -> C -> D -> E
   \-> F -> D -> E

When rebar is finished processing B, it restores the code path to what
it was before it processed B, removing C, D, E and G from the code path.
This means when it comes to process F, neither D or E are in the code
path, so any header includes, rebar plugins or parse transforms will not
be in the code path. Without the lib_dir directive, rebar does no code
path cleanups, so everything works fine.

This change makes rebar only remove the explicit lib_dir code paths it
added and adds an inttest that replicates the above scenario.
@Vagabond
Copy link
Contributor Author

cc @jaredmorrow

@Vagabond
Copy link
Contributor Author

Actually, to clarify, D will be added to the code path, but since D is now in the list of 'processed' directories, it will be 'skipped' and its deps won't be evaluated, so E will not be re-added to the code path.

@jaredmorrow
Copy link
Contributor

We have been running this code for a month in basho/rebar and has been tested aggressively.

jaredmorrow added a commit that referenced this pull request Nov 22, 2013
Don't over-aggressively clean the code path in the presence of lib_dir directives
@jaredmorrow jaredmorrow merged commit 9a158d3 into rebar:master Nov 22, 2013
jaredmorrow added a commit that referenced this pull request Dec 3, 2013
rebar_core: fix Dialyzer warning introduced in aa46d85 (#157)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants