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

Support .mesgignore #778

Merged
merged 3 commits into from
Feb 13, 2019
Merged

Support .mesgignore #778

merged 3 commits into from
Feb 13, 2019

Conversation

antho1404
Copy link
Member

For publication on the marketplace we need to be able to ignore some files, .dockerignore can be confusing to use and maybe some files needs to be shared to the marketplace but not to the build in docker so this is the implementation for a .mesgignore that is dedicated to ignore files for publication like a .npmignore for example

based on this discussion #755 (comment)

depends on #755

@antho1404 antho1404 changed the base branch from master to dev February 6, 2019 15:00
@krhubert
Copy link
Contributor

krhubert commented Feb 7, 2019

Can you explain a bit more what will be published on the marketplace and why we need .mesgignore?

@antho1404
Copy link
Member Author

The archive that the marketplace will receive needs to include all files that are necessary for the build or for the developer to run the service properly. The mesgignore is to remove all files that are not mandatory, node_modules, vendors, tmp... We could do that with the full archive but if files are not necessary then better to have a way to remove them.
We could have use the dockerignore file but for me dockerignore is really about the build and I don't want to reuse it for the deployment part, it serves different purposes even if they might be similar, like gitignore and dockerignore might be similar too

@krhubert
Copy link
Contributor

krhubert commented Feb 8, 2019

The archive that the marketplace will receive needs to include all files that are necessary for the build or for the developer to run the service properly.

So this is not what will be in deploy link on solidity marketplace?

The mesgignore is to remove all files that are not mandatory, node_modules, vendors, tmp...

the node_moduels, vendors etc will be downloaded in docker build so you wan't to exclude them, but they will be probably in dockerignore and gitignore.

I'm bit confused with it, can you give me an example with minimal repo and how this should look like:

  • what should be in the root directory
  • what should be in .mesgignore
  • what should be in .gitignore
  • what should be in .dockerignore
  • what will be in archive send to the marketplace

@antho1404
Copy link
Member Author

  • Archive in the marketplace will have everything in root directory minus files from the .mesgignore
  • Container created by the core will have everything in root directory minus files from .dockerignore
  • .gitignore is irrelevant here

@krhubert
Copy link
Contributor

krhubert commented Feb 8, 2019

I know how it will work, but can you give me a real example of small repo and what will be inside each file. What I'm asking is a simple use case of .mesgignore file that will be close to real service.

And possible not like this:

.dockerignore

node_modules
.mesgignore

node_modules

because they overlap. I'm asking of example where the .mesgignore will have diffrent content then .dockerignore, so use case for this.

@antho1404
Copy link
Member Author

I'm comparing this with npm and that when you install a package it can trigger a pre-install script that rely on files that are on the marketplace but not needed for the lib, just needed to build it.

For example having a script that generates a bin so we would have dockerignore that allows the bin and probably not allow the script that generates this bin and the mesgignore will do the opposite and ignore the bin and accept the script

Another use case could be with tests (we wanted to add a test command), we need the tests in the archive on the marketplace but the docker image generated don't need to contains them and just need the "production" version

I agree that these features are not here yet but I strongly feel that we will need some things like that and having a nice separation of files is anyway easier to understand and more flexible

@krhubert
Copy link
Contributor

krhubert commented Feb 11, 2019

I agree that these features are not here yet but I strongly feel that we will need some things like that and having a nice separation of files is anyway easier to understand and more flexible

So maybe let's wait for these features first, then we will have more clearness how we can use .mesgignore and know more usecases. If there will be features that .mesgignore can be used, then we can add this. To say, I'm not agains .mesgignore file at all, but at this moment as core can't make use of it.

@antho1404
Copy link
Member Author

I'm totally fine with that, the only "issue" is that maybe we will publish node_modules or vendors and this will make the archive big for nothing but I'm fine to have this first step first and then release this feature when actually needed.

@antho1404
Copy link
Member Author

@ilgooz @NicolasMahe what do you think about that ?

With .mesgignore or with nothing ?

@NicolasMahe
Copy link
Member

I suggest to have either this .mesgignore or use the .dockerignore to exclude files from the archive published to the marketplace.
If we start by reading the .dockerignore and then implement it with a .mesgignore then services will need to be updated and that's not a good idea.

So, i recommend to implement .mesgignore now.

@ilgooz
Copy link
Contributor

ilgooz commented Feb 12, 2019

@ilgooz @NicolasMahe what do you think about that ?

With .mesgignore or with nothing ?

I think it's good to have a .mesgignore file for the publish command because we may use ipfs in future and an ignore file can be useful to blacklist sensitive files, at least. And I also support your comment about the npm example.

@antho1404 antho1404 merged commit 4435404 into dev Feb 13, 2019
@antho1404 antho1404 deleted the feature/support-mesg-ignore branch February 13, 2019 02:46
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.

4 participants