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

Move the core comp unit repo to a separate folder #2940

Merged
merged 3 commits into from Jun 15, 2019

Conversation

patrickbkr
Copy link
Contributor

As proposed in this issue comment, move the core compilation unit repository previously installed directly into <Perl6 home>/ to <Perl6 home>/core. This should continue to work as before, but does not attempt to remove any old stale comp unit repository a previous install might have put there. So some cruft might be left behind.

@patrickbkr patrickbkr requested review from niner and ugexe May 31, 2019 17:25
@patrickbkr
Copy link
Contributor Author

I just noticed, that this branch is based on my reloc-fix branch and thus contains these changes as well. So please do not merge this branch prior to PR#2939.

@ugexe
Copy link
Member

ugexe commented May 31, 2019

I wonder if we could get away with switching the short name from perl to core. Its already poorly named since its parent folder is perl6...

$ perl6 -e 'say CompUnit::RepositoryRegistry.repository-for-name("perl")'
inst#/Users/ugexe/.rakudobrew/moar-blead-master/install/share/perl6

@patrickbkr
Copy link
Contributor Author

I'm not too familiar with the ins and outs of the repo stuff, but my guess would be the rename would probably work out without any fallout. @nine Do you have an opinion?

@ugexe
Copy link
Member

ugexe commented Jun 1, 2019

Only notable thing I can think of is:

CompUnit::Repository::Installation.new(
:prefix(@*ARGS[0]),
:next-repo(CompUnit::RepositoryRegistry.repository-for-name('perl')),
)

@patrickbkr
Copy link
Contributor Author

@ugexe, @niner I tried to also rename the repo from perl to core as per the suggestion of ugexe. I'm absolutely unsure about the changes I did for the rename (I know little about comp repos). It's possible I changed a 'perl' to 'core' that's unrelated to the repo name. Please do review.
Because the release is nearing it's probably a good idea to merge this as soon as possible, so we at least have some time left before the release.

Before it resided directly in <Perl6 home>. That was kind of messy as other
folders also reside in <Perl6 home> and are thus mixed in between the
folders of the CompUnit repo. The stale files of previous installs will not
be automatically removed, but will do no harm either.
See
rakudo#2919 (comment)
for the discussion surrounding this change.
tools/build/install-core-dist.p6 Outdated Show resolved Hide resolved
@patrickbkr
Copy link
Contributor Author

@ugexe, @niner I'd really like to have some kind of approval of this PR before merging, as I don't have much of an opinion on this change. Shall I merge?

@lizmat lizmat added the consensus needed Needs a well-versed decision with justification, possibly from a core developer label Jun 14, 2019
@patrickbkr
Copy link
Contributor Author

I'm merging this now to at least have a little time before the release. If someone has objections, just revert.

@patrickbkr patrickbkr merged commit 84fc659 into rakudo:master Jun 15, 2019
@patrickbkr patrickbkr deleted the core-comprepo branch June 15, 2019 10:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
consensus needed Needs a well-versed decision with justification, possibly from a core developer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants