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

Fix Issue #14058 -- 'make install' should not copy non-source files #2918

Closed
wants to merge 1 commit into from

Conversation

WebDrake
Copy link
Contributor

This patch ensures that make install will copy only actual phobos files to the install src/ dir and not (for example) backup files left behind by code editors.

This patch ensures that 'make install' will copy only actual phobos
files to the install src/ dir and not (for example) backup files left
behind by code editors.
@WebDrake
Copy link
Contributor Author

Dammit, I was worried about that -i option. I'll come back to this later in the day.

@quickfur
Copy link
Member

According to the manpage, you should use -I{} instead of -i.

cp -r std/* $(INSTALL_DIR)/src/phobos/std/
cp -r etc/* $(INSTALL_DIR)/src/phobos/etc/
mkdir -p $(INSTALL_DIR)/src/phobos
find std -type f -name "*.d" | xargs -n1 -i cp -r --parents {} $(INSTALL_DIR)/src/phobos/
Copy link
Member

Choose a reason for hiding this comment

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

This will break when we add some .di files to Phobos.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I know. The original version did have an -o -name "*.di" extra in there, but I removed it when I realized there weren't actually any .di files at present. Anyway, in light of your other comment, this should be rethought anyway ;-)

@andralex
Copy link
Member

Using wildcards in makefiles is a bit of an antipattern. We have a list of all Phbos source modules. We should use that.

@WebDrake
Copy link
Contributor Author

According to the manpage, you should use -I{} instead of -i

Yup. I didn't manage to get that working this morning (I was in a rush and so switched to -i because it Just Worked), but I'll try and revise accordingly. (Might as well get a working wildcard-based version just as a proof of concept, before I switch to using existing sources list.)

@andralex
Copy link
Member

@WebDrake thx, looking forward to the new coolness

@quickfur
Copy link
Member

ping
It's been a while. Any updates on this PR?

@DmitryOlshansky
Copy link
Member

@WebDrake ? Going to put this to rest in a couple of weeks.

@WebDrake
Copy link
Contributor Author

WebDrake commented Sep 4, 2015

I'm sorry to say that it's unlikely I'll follow up in the next 2 weeks. I can always reawaken this once I have time, so if it simplifies matters for you, please do feel free to close it sooner or later.

@DmitryOlshansky
Copy link
Member

@WebDrake we have bugzilla and I generally try to leave links to prior work when closing stuff. And yes one less pull with limbo status is always simpler to manage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants