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

Add installation script in install.sh #206

Merged
merged 87 commits into from
Oct 27, 2020

Conversation

rouson
Copy link
Contributor

@rouson rouson commented Oct 13, 2020

No description provided.

LKedward and others added 28 commits September 28, 2020 09:44
Adds recursive source discovery for local path dependencies
Now supported with local dev-dependencies
Local dependency paths are relative to the dependent package not the building package.
There's a bug which causes app-local modules to be added twice if auto-discovery is on and the app is specified in the manifest. This causes the module to be compiled and linked twice. Not detected before because the module contained no symbols. This commit adds an integer symbol to an app-local module to test this bug.
Demonstrates bug in include statement parsing - currently erroneously parsing all statements that begin with 'include'.
Include statements must have a single or double quote immediately following 'include'
Demonstrates bug in include statement parsing - currently erroneously parsing all statements that begin with 'include'.
Include statements must have a single or double quote immediately following 'include'
Co-authored-by: Milan Curcic <caomaco@gmail.com>
@certik
Copy link
Member

certik commented Oct 13, 2020

Thanks @rouson for sending it. I think this only works on Linux?

@rouson
Copy link
Contributor Author

rouson commented Oct 13, 2020

@certik it was developed on macOS. The goal will be for it to also work on Linux (I'll test it on Lubuntu) and on the Windows Subsystem for Linux (which I'll eventually test too but I'm not sure of the timeline as I need to purchase Windows). Also, this first version is skeletal. Over time, I'll attempt to make it more robust and add features. For example, I'll add

  • --help flag
  • the option to specify an install path
  • use of sudo for installation paths that require it

If you have other requested features, let me know and feel free to submit issues.

In theory, I can borrow a lot from the OpenCoarrays installer, but that installer is written in bash 3 because it was the default macOS shell until recently. Now the default macOS shell is zsh and @everythingfunctional explained to me that the default on Linux is dash, of which I never heard until yesterday. If possible, I'll go with dash because it's POSIX-compliant, but that depends on how much I'm able to borrow or convert from the OpenCoarrays installer. It also depends on whether one can expect to find dash preinstalled on most platforms across each of the aforementioned operating systems.

Minor fixes: to list_files and mkdir in fpm_filesystem
…ap_submodule_support

Bootstrap submodule support
Co-authored-by: Brad Richardson <everythingfunctional@protonmail.com>
@everythingfunctional everythingfunctional merged commit a22ce1c into fortran-lang:master Oct 27, 2020
@certik
Copy link
Member

certik commented Oct 27, 2020

Hold on, I think a mistake was made in this PR. It seems to include tons of unrelated changes. @everythingfunctional did you merge more things by a mistake? When you update a PR, please ask for another review. We should be merging PRs with a nice git history that only do one thing. This PR, at least according to GitHub is mixing together many unrelated changes. It could also be just a mistake at GitHub, but in that case we should simply rebase on top of the latest master to ensure the PR is small and in this case only includes the install.sh script.

@everythingfunctional
Copy link
Member

I think what Github is now showing after-the-fact is not correct. (I'm pretty sure) when I clicked merge, the only change showing was the addition of the install.sh script. There were no changes made, only merging in the master branch to make sure there were no conflicts (which was admittedly unnecessary).

@certik
Copy link
Member

certik commented Oct 27, 2020

Before you merged, master was at the commit: 4443986, which is fine. This PR was at a commit cda71a0, and when you clicked "Merge", GitHub added the merge commit a22ce1c, which is now the latest master.

If you look at cda71a0 (i.e., this PR) and the history in there, this is what I see:

*   cda71a0 (HEAD) Merge branch 'installer' of github.com:sourceryinstitute/fpm into installer
|\  
| * b44b567 fix(install.sh): define install_path earlier
| * de96c4a Update install.sh
| * 7eca78c add skeletal installer
| * d64e54a WIP: start installation script
* | e331951 fix(install.sh): define install_path earlier
* | 108a997 Update install.sh
* | 6624b64 add skeletal installer
* | 70b25be WIP: start installation script
* |   4443986 Merge pull request #213 from everythingfunctional/bootstrap_submodule_support
|\ \  
| * | 488bdd0 Add .gitignore file in submodules example package
| * | 03c9efc Add test with submodule example project and fix .smod naming convention
| * | f196336 Fix CI scripts
| * |   39cca4d Merge branch 'master' into bootstrap_submodule_support
| |\ \  
| * | | e7b135b convert buildDirectory path to native in buildProgram
...

So it seems it is not clean --- the commits are there twice, such as b44b567 and e331951, etc. I don't quite understand how it happened, but it does not seem right.

Anyway, we can probably keep it, since the alternative is to revert this.

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.

7 participants