Skip to content

Add function #'ADD-LOCAL-PROJECTS-TO-MANIFEST #110

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

Merged
merged 7 commits into from
Dec 21, 2014
Merged

Add function #'ADD-LOCAL-PROJECTS-TO-MANIFEST #110

merged 7 commits into from
Dec 21, 2014

Conversation

EuAndreh
Copy link
Contributor

Fixes #83

I tried to copy the same ideas of pathname handling and stream writing from other functions of the quicklisp-client package.

Maybe add to the docstring a tip saying to use (ql:write-asdf-manifest-file) and (ql:register-local-projects) before using (ql:add-local-projects-to-manifest) in order to keep all up-to-date?

@quicklisp
Copy link
Owner

Thanks for working on this. I think I'd like slightly different behavior:

  • Local project systems should be added to the manifest automatically
  • A new keyword argument, :exclude-local-projects, should exclude those projects
  • The local projects should take precedence over quicklisp built-in projects

For that final point, I don't know if that means the local-projects must precede or succeed the Quicklisp-provided systems...double check the manifest file code in buildapp to be sure.

Do you feel like revising your code to do that? If not, I can look into doing it myself this weekend.

@quicklisp
Copy link
Owner

The new keyword argument should be added to write-asdf-manifest-file and default to nil.

@EuAndreh
Copy link
Contributor Author

Sure, I'll do it.

Thanks for your review.

@EuAndreh
Copy link
Contributor Author

Still need to check the precedence of local systems. Still working on that.

@EuAndreh
Copy link
Contributor Author

The function (asdf-directive-files '((:manifest-file "manifest/file/path.txt")), returns the system in the same order as in the manifest file. All functions that call it and beyond don't reverse the result.
Thus, if local-projects systems are first in the manifest file, they get precedence.

Did I implement it properly?
Is that all?

@quicklisp
Copy link
Owner

One last thing - if exclude-local-projects is nil, call register-local-projects before saving the manifest?

@EuAndreh
Copy link
Contributor Author

Done

@quicklisp
Copy link
Owner

Thank you, things look great! There's one last thing that I noticed that might be a potential issue.

Entries in the manifest file should be written in relative form if the manifest file is in a parent directory of the system file. I use enough-namestring for that in write-asdf-manifest-file. Could you please make sure manifest entries are relative if applicable?

@EuAndreh
Copy link
Contributor Author

Sure, I'm glad I can help.

@EuAndreh
Copy link
Contributor Author

I used the same variable names as the original code.
I didn't put the verification (and system-file (enough-namestring system-file output-file)) because the generated list is already "valid" (withou nil atoms).

quicklisp added a commit that referenced this pull request Dec 21, 2014
Extend WRITE-ASDF-MANIFEST-FILE to consider local-projects systems.
@quicklisp quicklisp merged commit b679f3d into quicklisp:master Dec 21, 2014
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.

ql:write-asdf-manifest-file does not include local projects
2 participants