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

Use Dir.glob instead of git ls-files #804

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

utkarsh2102
Copy link

@utkarsh2102 utkarsh2102 commented May 21, 2020

Hi,

Thanks for working on whenever so far.
However, whilst maintaining this in Debian, I found it to have an unnecessary dependency on git whereas that could be very well avoided by using Dir.glob.

This PR fixes the same :)

@benlangfeld
Copy link
Collaborator

The dependency on git should only be for packaging the gem, not using it.

@utkarsh2102
Copy link
Author

utkarsh2102 commented May 21, 2020

Hi @benlangfeld,

The dependency on git should only be for packaging the gem, not using it.

That's right. I maintain whenever in Debian :)
And we need to patch out git to avoid an unnecessary dependency and it'd be very nice (and kind) if this could be updated here, in upstream itself 😄

Hope this makes sense and could be merged 🚀

@benlangfeld
Copy link
Collaborator

Hi @benlangfeld,

The dependency on git should only be for packaging the gem, not using it.

That's right. I maintain whenever in Debian :)

And we need to patch out git to avoid an unnecessary dependency and it'd be very nice (and kind) if this could be updated here, in upstream itself 😄

Hope this makes sense and could be merged 🚀

I think you misunderstood what I said. You shouldn’t need a git dependency at runtime if you’re getting them gem from rubygems.org. If you’re pulling the source code then you’re doing it wrong.

@utkarsh2102
Copy link
Author

Hi,

I think you misunderstood what I said. You shouldn’t need a git dependency at runtime if you’re getting them gem from rubygems.org. If you’re pulling the source code then you’re doing it wrong.

I think I was not clear. I'll try to be more verbose :)
When I say I maintain this gem in Debian -- that means, I packaged it for apt repositories.
While doing so, I need to build it from source and run tests to make sure that everything's alright. I can't just pull it from anywhere. And while building it from source, I hit this obstacle where I needed to patch the usage of git altogether. I hope that makes sense now? 😄

Also, what I don't understand is why would you be not interested in using Dir.glob instead of git? Is there any particular reason?
(I've patched the same thing in multiple gems, none seem to have this problem and were happily merged. So I am curious to know if there's anything blocking you in doing so?)

@benlangfeld
Copy link
Collaborator

Hi,

I think you misunderstood what I said. You shouldn’t need a git dependency at runtime if you’re getting them gem from rubygems.org. If you’re pulling the source code then you’re doing it wrong.

I think I was not clear. I'll try to be more verbose :)

When I say I maintain this gem in Debian -- that means, I packaged it for apt repositories.

I know very well what this popular phrase means.

While doing so, I need to build it from source and run tests to make sure that everything's alright. I can't just pull it from anywhere. And while building it from source, I hit this obstacle where I needed to patch the usage of git altogether. I hope that makes sense now? 😄

This project is distributed via rubygems.org. Your project is based on not liking Rubygems and choosing to instead use your own packaging and distribution format. You choose to behave as a developer of the project, rather than a consumer of it. Git is a development dependency of the project. You therefore require git. If you instead behave as a consumer, by fetching an official release of the project from rubygems.org, you don’t inherit the dependency on git.

Also, what I don't understand is why would you be not interested in using Dir.glob instead of git? Is there any particular reason?

Git is used here to ensure that only tracked files, and not temporary ones which may be generated as part of the build process or otherwise such as log files, are included in the released gem. A glob cannot ensure this.

(I've patched the same thing in multiple gems, none seem to have this problem and were happily merged. So I am curious to know if there's anything blocking you in doing so?)

@utkarsh2102
Copy link
Author

utkarsh2102 commented May 22, 2020

This project is distributed via rubygems.org. Your project is based on not liking Rubygems and choosing to instead use your own packaging and distribution format. You choose to behave as a developer of the project, rather than a consumer of it. Git is a development dependency of the project. You therefore require git. If you instead behave as a consumer, by fetching an official release of the project from rubygems.org, you don’t inherit the dependency on git.

You are wrong here. We do pull it from rubygems and then add a debian/ directory to it.
There's this tool called gem2deb, which pulls the gem from rubygems.org and converts it into a Debian package.
When run, it pulls all the files that you ship in the gem release and builds on top of it. Of course, it also pulls the gemspec with git in it :)
This is why we need to patch it.

And I'm worried but we don't behave as "developers". We're just packaging it so it reaches more "consumers" (in different forms) from its upstream "developers", that is you :)
Hence this request.

Git is used here to ensure that only tracked files, and not temporary ones which may be generated as part of the build process or otherwise such as log files, are included in the released gem. A glob cannot ensure this.

Fair enough. Then would you be okay if we include only those files which matter in the release?
Like: Dir["lib/**/*", "test/**/*", "bin/*", "LICENSE"]? Would you be okay with this?
I appreciate and understand what you're saying but it'd be a whole lot nicer if this can be integrated here :)

@benlangfeld
Copy link
Collaborator

benlangfeld commented May 22, 2020

When run, it pulls all the files that you ship in the gem release and builds on top of it. Of course, it also pulls the gemspec with git in it :)
This is why we need to patch it.

At what point do you execute the contents of the gemspec? Within gem2deb? What is the problem with the machine which executes gem2deb requiring git to be installed (which is included in the gem2deb documentation)? You shouldn't have to specify git as a dependency of the resulting dpkg, which I agree would be onerous.

@utkarsh2102
Copy link
Author

I added more comments to my previous comment: #804 (comment). Can you take a look and reply to them as you see fit? :)

At what point do you execute the contents of the gemspec? Within gem2deb? What is the problem with the machine which executes gem2deb requiring git to be installed (which is included in the gem2deb documentation)? You shouldn't have to specify git as a dependency of the resulting dpkg, which I agree would be onerous.

Oh, you got the wrong wiki, hehe :)
But nevermind, I can explain you the entire procedure.

  • We get the skeleton Debian package by gem2deb which is based on pulling files and data shipped in the gem's release.
  • This takes the files shipped and adds a debian/ directory to it -- for preparing the Debian package.
  • Next, we work on running upstream tests, adding your LICENSE to it, and other things to check if there's nothing wrong and give a quick look at the things pulled.
  • Next, we build the package via dpkg-buildpackage -uc -us which does the initial build.
  • Next comes the role of a static analyzer which helps us in determining if there's anything wrong.
  • When this is all done, we do a final build in a clean environment, schroot or sbuild. This means building a package in a clean container with only having the gem's build dependencies and nothing extra.

Now, of course, we ourselves use git to manage the packaging repositories. But we cannot add git as a dependency to it as you've guessed why.
Since we do not add git as a dependency, the build in the clean container fails when it sees the usage of git but not having git installed. Therefore, making the package not build-able.
To avoid this, we generally patch the usage of git in the way such that both upstream maintainers are happy and so are we. We respect the work you do and we'd like to help you as much as possible.
For it to happen, we'd expect a little help from you, too, in the form of asking you to adapt a way to avoid git in gemspec in such a way that is not only right but with which, both of us benefits :)

@benlangfeld
Copy link
Collaborator

Why do you expect a build of source code to work in an environment that doesn’t include that code’s development dependencies? If you’re shipping a compiled C++ project, does that build container not include a compiler?

@utkarsh2102
Copy link
Author

Of course it has all the dependencies in the environment. Just not git.
Would you want to add git to your gemspec? No, I suppose. That's what I mean. We can very well achieve the same functionality without using git in gemspec. It's an unnecessary usage, really.

And for the latter part, of course. We don't ship compiled stuff but build them during our package build. And of course, yes.

@benlangfeld
Copy link
Collaborator

Git is a dependency of this project in the same way that gcc or similar is a dependency of a C project. I don’t understand why that is controversial.

@utkarsh2102
Copy link
Author

I am very surprised that you said this.
Are you by any chance comparing what gcc is to C, the same is git to this project?

D'you need git for doing any compilation work or anything? I don't think so.
I'm not only surprised but shocked to see this resistance for something that can be done using plain Ruby. I'm willing to understand your point if you give me a valid explanation as to why can't you drop git and use some other alternative for doing the same work. I can further list 3 more alternatives that you can adapt to if you don't want to use Dir.glob.

@benlangfeld
Copy link
Collaborator

This has got petty and uncooperative. I’m done. Thank you for your suggestion.

Base automatically changed from master to main January 20, 2021 18:17
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.

None yet

2 participants