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

Fix sunwait-src submodule initialisation issues #362

Closed
wants to merge 3 commits into from

Conversation

IanLauwerys
Copy link
Contributor

I might be barking up the wrong tree here, so feel free to reject this if so. I've been having some issues with the project, specifically using GitKraken as the Git client. I can reproduce the issue consistently as follows:

  1. Fork thomasjacquin/allsky to ianlauwerys/allsky directly on github.com
  2. Clone the ianlauwerys/allsky repo to my local machine using GitKraken
  3. GitKraken is unable to initialise the sunwait-src submodule and throws an error whenever I try
  4. This then leads to various errors and issues trying to create or switch branches due to the uninitialized submodule.

The error that GitKraken throws up when trying to initialise the sunwait-src submodule is that sunwait-src already exists in the index. On investigation I believe what has happened is that the https://github.com/risacher/sunwait repo was added to allsky as a submodule under the sunwait-src directory, but that this directory and its contents already existed from earlier development prior to (re-)adding the same code as a submodule.

When transitioning from using some external source files directly in a repo to adding them as a submodule, one should first remove the source files and unstage them before adding and initialising the submodule. As far as I can tell this wasn't done, so adding the submodule creates a conflict with the directly added source files from earlier work. I may be wrong about this as I am far from a git expert so feel free to set me straight.

There seems to be no way forward to resolve this issue other than what I have done in this branch:

  1. Remove the sunwait submodule via git.
  2. Commit and push the changes.
  3. Remove the sunwait-src directory and its contents.
  4. Stage, commit and push the changes.
  5. Re-add the sunwait submodule to sunwait-src.
  6. Initialise the submodule.
  7. Stage, commit and push the changes.

My repo is now in a sane state and able to create and switch branches without errors. I was previously using SourceTree and didn't see the same issues, but I'm not sure how capable or tolerant SourceTree is when dealing with submodules vs. GitKraken.

The only concern is that the sunwait submodule is initialised from the current state of its repo which is some way ahead of the current directly included source. As far as I can see the usage of the main sunwait executable hasn't changed, but before merging it would be wise to check for any conflicts in the current installation script and make files - I think they are OK and haven't changed anything, but you're the expert!

Cheers

@EricClaeys
Copy link
Collaborator

Is this still needed? If not, should it be closed?

@ckuethe
Copy link
Collaborator

ckuethe commented Sep 29, 2021

Here's a crazy idea.. I wonder if, instead of shelling out to run sunwait, it could be turned into a library that capture links against. Then the calculation could be run by a thread. It looks kind of like sunriset.cpp does most of what we need.

@linuxkidd
Copy link
Collaborator

sounds great to me!

@joxda
Copy link
Contributor

joxda commented Sep 29, 2021

Check whether you'll find https://github.com/joxda/libsunwait useful.

@ckuethe
Copy link
Collaborator

ckuethe commented Sep 29, 2021

Yep, that's pretty much exactly what was thinking of. Thanks for the tip.

@linuxkidd linuxkidd mentioned this pull request Oct 2, 2021
@linuxkidd
Copy link
Collaborator

Closing as resolved by #535

Although, we will cerainly move to the sunwait library very soon. Thanks!

@linuxkidd linuxkidd closed this Oct 3, 2021
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.

5 participants