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

Moved the K submodule into the deps directory #427

Closed
wants to merge 4 commits into from

Conversation

charala1
Copy link
Contributor

@charala1 charala1 commented May 28, 2019

modified:

  • Dockerfile
  • Makefile
  • .gitmodules
  • .build/k -> deps/k

modified:
 - `Dockerfile`
 - `Makefile`
 - `.gitmodules`
 - `.build/k` -> `k`
@charala1 charala1 requested a review from dwightguth May 28, 2019 18:44
@dwightguth
Copy link
Member

@ehildenb what do you think about this change? I tend to agree with Minas that it's better not to have dependency submodules in a hidden directory that also contains build outputs. But I don't want to make this change unless we commit to changing it consistently.

@charala1
Copy link
Contributor Author

charala1 commented May 28, 2019

@ehildenb what do you think about this change? I tend to agree with Minas that it's better not to have dependency submodules in a hidden directory that also contains build outputs. But I don't want to make this change unless we commit to changing it consistently.

@dwightguth The c-semantics submodule is not in a hidden directory in rv-match, but the k back-ends are in hidden directories inside the k project folder. I agree this change should be propagated everywhere.

@ehildenb
Copy link
Member

@dwightguth I'm not opposed to this change, but I don't think I'll make it for KEVM and KWasm unless there is a good quantitative argument about how it makes dev-work easier. I personally like limiting how many things are stored directly in the root of the repo because I like to show people the repo on GitHub during demos (so we get nice formatting), and I think it gets confusing to do that when there are lots of extra files.

I would be open to changing it to a non-hidden directory called deps if you like. I do not think it's worth the effort to do out-of-tree builds for our dependencies. For me doing builds in a file that is .gitignoreed counts as "out-of-tree".

@ehildenb
Copy link
Member

I am opposed to any solution which does builds outside the repository root directory though, because I am opposed to cluttering any other directories on the users system.

@charala1
Copy link
Contributor Author

charala1 commented May 28, 2019

I am opposed to any solution which does builds outside the repository root directory though, because I am opposed to cluttering any other directories on the users system.

Just to clarify, I am opposing this too -- except as directed by the user. Much like autotools uses --prefix and/or the make PREFIX variable, or how using cmake entails cd some/dir && cmake /path/to/src && make.

@dwightguth
Copy link
Member

If we're not going to change kevm, we should not change c-semantics. That said, I like the idea of dependency submodules living in a deps directory under the root better than them living under a .build directory. Is this something we can agree to propagate across all our semantics? I think it makes the repository significantly easier to explain if we separate build artifacts from source code in the directory structure, even if it adds an extra directory.

@charala1
Copy link
Contributor Author

If we're not going to change kevm, we should not change c-semantics. That said, I like the idea of dependency submodules living in a deps directory under the root better than them living under a .build directory. Is this something we can agree to propagate across all our semantics? I think it makes the repository significantly easier to explain if we separate build artifacts from source code in the directory structure, even if it adds an extra directory.

I agree, placing things inside a new directory deps/ or external-deps/ or similar makes things much more obvious and clean.

@charala1 charala1 changed the title Moved the K submodule into the root directory Moved the K submodule into the deps directory May 28, 2019
@ehildenb
Copy link
Member

@dwightguth I can agree to move dependencies to a deps directory and continue to do builds in the .build directory (or in a build directory).

@charala1 charala1 closed this Jun 28, 2019
@charala1 charala1 deleted the minas-k-submodule branch June 28, 2019 16:04
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

3 participants