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.

Sign up for GitHub

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

Jump to bottom

git-svn: Separate it from git #87241

Closed
wants to merge 2 commits into from
Closed

git-svn: Separate it from git #87241

wants to merge 2 commits into from

Conversation

moonfruit
Copy link
Contributor

@moonfruit moonfruit commented Oct 14, 2021 •
edited
Loading

  • Have you followed the guidelines for contributing?
  • Have you ensured that your commits follow the commit style guide?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
  • Does your build pass brew audit --strict <formula> (after doing brew install --build-from-source <formula>)? If this is a new formula, does it pass brew audit --new <formula>?

Separate git-svn from git to solve #52490. @iMichka @Bo98

carlocab
carlocab reviewed Oct 14, 2021
View reviewed changes
Formula/git-svn.rb Show resolved Hide resolved
Formula/git-svn.rb Show resolved Hide resolved
Formula/git-svn.rb Outdated Show resolved Hide resolved
Formula/git-svn.rb Outdated Show resolved Hide resolved
Formula/git-svn.rb Outdated Show resolved Hide resolved
Formula/git.rb Show resolved Hide resolved
@carlocab
Copy link
Member

Thanks very much for working on this, @moonfruit.

@carlocab carlocab added the CI-no-fail-fast Continue CI tests despite failing GitHub Actions matrix builds. label Oct 14, 2021
@carlocab
Copy link
Member 

==> git svn clone file:///private/tmp/git-svn-test-20211014-83971-bqp1vy/repo git-work
Can't locate Git/SVN.pm in @INC (you may need to install the Git::SVN module) (@INC contains: /opt/homebrew/opt/git/share/perl5 /opt/homebrew/opt/subversion/lib/perl5/site_perl/5.30.2/darwin-thread-multi-2level /Applications/Xcode.app/Contents/Developer/Library/Perl/5.30/darwin-thread-multi-2level /Library/Developer/CommandLineTools/Library/Perl/5.30/darwin-thread-multi-2level /Library/Perl/5.30/darwin-thread-multi-2level /Library/Perl/5.30 /Network/Library/Perl/5.30/darwin-thread-multi-2level /Network/Library/Perl/5.30 /Library/Perl/Updates/5.30.2 /System/Library/Perl/5.30/darwin-thread-multi-2level /System/Library/Perl/5.30 /System/Library/Perl/Extras/5.30/darwin-thread-multi-2level /System/Library/Perl/Extras/5.30) at /opt/homebrew/bin/git-svn line 23.
BEGIN failed--compilation aborted at /opt/homebrew/bin/git-svn line 23.

@moonfruit
Copy link
Contributor Author

Does formula git is installed in the ci container? @carlocab

@carlocab
Copy link
Member

Does formula git is installed in the ci container?

It is not. You need to do depends_on "git" if you want the git formula to be installed.

@moonfruit moonfruit force-pushed the git-svn-formula branch 3 times, most recently from 11f055a to c578c94 Compare October 14, 2021 12:39
carlocab
carlocab reviewed Oct 14, 2021
View reviewed changes
Formula/git-svn.rb Outdated Show resolved Hide resolved
@moonfruit
Copy link
Contributor Author 

git-svn:
  * Don't use 'git' as a dependency (it's always available)

This is why I do not depend on git at first.

Bo98
Bo98 reviewed Oct 14, 2021
View reviewed changes
Copy link
Member

@Bo98 Bo98 left a comment

Choose a reason for hiding this comment

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

I'm quite happy with this approach - thank you!

Formula/git-svn.rb
Comment on lines +26 to +34
if OS.mac?
ENV["PERLLIB_EXTRA"] += ":" + %W[
#{MacOS.active_developer_dir}
/Library/Developer/CommandLineTools
/Applications/Xcode.app/Contents/Developer
].uniq.map do |p|
"#{p}/Library/Perl/#{perl_short_version}/darwin-thread-multi-2level"
end.join(":")
end
Copy link
Member

Choose a reason for hiding this comment

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

Is the intention here to use system subversion modules in some cases? Because the depends_on "subversion" prevents that from ever happening.

Copy link
Contributor Author

@moonfruit moonfruit Oct 14, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

No, here I just want to be the same with git's other perl commands.

Copy link
Member

Choose a reason for hiding this comment

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

This was to make use of system SVN Perl bindings and the directory does not exist anymore.

Formula/git-svn.rb Show resolved Hide resolved
@Bo98
Copy link
Member

Bo98 commented Oct 14, 2021

This is why I do not depend on git at first.

This audit is only for new formula so we can ignore that for this and it won't pop up again.

@moonfruit
Copy link
Contributor Author

Is there something to do about this? @Bo98

@Bo98 Bo98 added the new formula PR adds a new formula to Homebrew/homebrew-core label Oct 21, 2021
@BrewTestBot
Copy link
Member

:shipit: @iMichka has triggered a merge.

@iMichka
Copy link
Member

iMichka commented Oct 22, 2021

Thanks @moonfruit ! Nice work here.

@moonfruit moonfruit deleted the git-svn-formula branch October 26, 2021 07:06
@github-actions github-actions bot added the outdated PR was locked due to age label Nov 26, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 26, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Bo98 Bo98 Bo98 approved these changes

@iMichka iMichka iMichka approved these changes

@carlocab carlocab Awaiting requested review from carlocab

Assignees
No one assigned
Labels
CI-no-fail-fast Continue CI tests despite failing GitHub Actions matrix builds. new formula PR adds a new formula to Homebrew/homebrew-core outdated PR was locked due to age
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@moonfruit @carlocab @Bo98 @BrewTestBot @iMichka