eventide-project / consumer

Event Sourcing and Microservices Stack for Ruby
http://eventide-project.org
Other
1 stars 3 forks source link

Prevent symlink files from consumer-postgres from being inadvertently included in gem file #11

Closed ntl closed 4 years ago

ntl commented 4 years ago

For context: this project has historically included lib/consumer/postgres.rb and lib/consumer/postgres in its .gitignore file. This is because, when both the Consumer and Consumer::Postgres libraries are symlinked (i.e. with ./symlink-lib.sh), the Consumer::Postgres symlinks get placed inside this project's lib/consumer directory, and we need to prevent those symlinks from ever being inadvertently added to the project. This is simply how things have to be when symlinking multiple projects that share a common namespace (the way that Consumer and Consumer::Postgres share Consumer as a common namespace).

This change addresses another issue that stems from having Consumer::Postgres symlinks placed in lib/consumer during development: when packaging the gem file, the lib/consumer/postgres.rb and lib/consumer/postgres symlinks are getting inadvertently added to the gem package. To counteract this, I changed the gemspec to exclude those symlinks from the gem. The specific symlinks excluded are the same ones that are in .gitignore.

I'm also hoping we can settle this matter once and for all, so that all the libraries like Consumer that can have symlinks from "inner" libraries like Consumer::Postgres get fixed. However we agree to solve the problem here, I'll introduce across the board.

sbellware commented 4 years ago

Ideally, the reject clause would select based on whether the file or directory is a symlink. Presuming Ruby's file utilities allow for detection of symlinks, it seems like that would be a more comprehensive approach.

ntl commented 4 years ago

@sbellware done. Do you approve the current code change? Once final, I believe it should go in every gemspec, not just the ones like Consumer that can happen to have the problem. (Please correct me on this if I'm wrong)

sbellware commented 4 years ago

@ntl I think this could also be done by putting the safeguards in the packaging scripts. Having excludes in the gemspec means that vestiges of development process are showing up in packaging artifacts. That's also an undesirable couple.

ntl commented 4 years ago

I agree with that, I had a similar thought. By "packaging script" are you referring to something contained in one of our projects, or just the scripting we write for ourselves as developers?

sbellware commented 4 years ago

@ntl For me, it's a matter of updating the single bash alias that builds gems, and amending it to remove the LIBRARIES_HOME directory before creating any packages. So yes, each person who has the authority to publish a package must update their own tooling. Although we could put a standardized script in contributor_assets for building a gem.

ntl commented 4 years ago

I don't mind taking both approaches; what's in contributor_assets could be useful to provide for new contributors who may not be used to leveraging their own tooling for things like this.

sbellware commented 4 years ago

I created this issue in contributor_assets for the build script solution: https://github.com/eventide-project/contributor-assets/issues/2

ntl commented 4 years ago

OK. I'll work on that script next, then. Any reason I shouldn't close this PR?

sbellware commented 4 years ago

@ntl No reason that I can see not to close the PR. I'll just do it now while I'm here.