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

Add dynamic_bin_dir method #191

Merged
merged 3 commits into from
May 5, 2020
Merged

Conversation

shawnlaffan
Copy link
Contributor

@shawnlaffan shawnlaffan commented May 3, 2020

[WIP]

As discussed in PerlAlien/Alien-curl#10 (comment)

Still to do:

  • It needs tests, but I'm not entirely sure where they should go.
  • The system case is identical to that for bin_dir. Should it be? Or is this likely to cause problems when system libs are statically compiled? (Although there is no control at this level over system libs being static or dynamic).

@plicease
Copy link
Member

plicease commented May 3, 2020

* [ ]  It needs tests, but I'm not entirely sure where they should go.

Tests should go in t/alien_base.t, ideally as a new subtest. May need to create a new mock alien in the corpus directory.

* [ ]  The system case is identical to that for bin_dir.  Should it be? Or is this likely to cause problems when system libs are statically compiled?  (Although there is no control at this level over system libs being static or dynamic).

I am undecided on this point tbh. I am also not sure if this method shouldn't be called dynamic_dir.

@shawnlaffan
Copy link
Contributor Author

shawnlaffan commented May 4, 2020

Thanks. Will look at the other tests in alien_base.t and adapt.

I'm also leaning towards dynamic_dir as the method name now you raise it.

In theory one could have an entire structure under there if it is set as the build target in the first place, so .../dynamic/bin, .../dynamic/include etc. That would have back-compat issues, though.

@shawnlaffan
Copy link
Contributor Author

The two updates in a072676 and ce9723b change the method name to dynamic_dir and add tests. They also set the method to return the empty list when called on a system install - let me know if this needs to be changed.

@plicease plicease merged commit b86fb33 into PerlAlien:master May 5, 2020
@shawnlaffan shawnlaffan deleted the dynamic_bin_dir2 branch May 5, 2020 21:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants