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

Star6 #94

Closed
wants to merge 0 commits into from
Closed

Star6 #94

wants to merge 0 commits into from

Conversation

perevbnlgov
Copy link
Contributor

There is Star6 version of star-sw. Now it is compiled by Cons.
Victor

@plexoos
Copy link
Member

plexoos commented Aug 11, 2021

This PR looks almost identical to #59 except you are trying to merge perevbnlgov:Star6 into star-bnl:Star6

I think what you intended to do was merging star-bnl:Star6 into star-bnl:main

@perevbnlgov
Copy link
Contributor Author

perevbnlgov commented Aug 11, 2021 via email

@plexoos
Copy link
Member

plexoos commented Aug 11, 2021

No, I update branch Star6.

In this case, some of the previous comments and suggestions we discussed in #59 and during the meetings weeks ago still remain unaddressed. For example, why do you still propose to bring StRoot/StarGenerator/Pythia8_1_62 into this repository while this package is already in https://github.com/star-bnl/star-mcgen? How is it related to our ability to use ROOT6 in star-sw?
For your information:

  • star-sw compiles just fine without any code in star-mcgen
  • Our current CI jobs already build almost entire star-sw against both ROOT5 and ROOT6
  • You should update your branches with the latest improvements in CI by either merging the main branch into your branches or rebasing your branches onto main

@perevbnlgov
Copy link
Contributor Author

perevbnlgov commented Aug 11, 2021 via email

@veprbl
Copy link
Member

veprbl commented Aug 11, 2021

@perevbnlgov
I have a suggestion on how you could clean up the mess. Starting with your Star6 branch you can do git reset --soft HEAD~2. The meaning of that is to remove the 2 commits that you have (at this point git log will should show an old version of the main branch that you've branched off, and git status will show contents of your two commits staged). Then you do git reset to unstage (at this point git status will show bunch of unstaged changes). You can then run git add -p that would allow you to hand pick changes to stage. This will iterate through files that were already in the main branch, if you need to stage a new file, use git add path/to/individual/file.cxx. Once you've satisfied you can check what changes have been staged for commit by running git diff --staged and what changes will be left out of it by doing git diff and git status. You can then call git commit and only the changes that you've staged would make it into a commit (run git show to confirm that). Repeat git add -p/git commit until you create reasonable set of commits.

Overall you should never use git add --all/git add ., that is like flying blind.

@perevbnlgov
Copy link
Contributor Author

perevbnlgov commented Aug 11, 2021 via email

@veprbl
Copy link
Member

veprbl commented Aug 11, 2021

As I understand, you cleanup the local repository and then add modifications.

Yes. The suggestion is to give up on the existing commits and redo them from scratch with the help of git.

But I already did in bu different way. I clone the official repository in Github (star-bnl/star-sw) into local repository. This repository is clean from any my additions, and then push it into my forked repository in GitHub. Is it the same?

This does not hurt, but doesn't help in this case.

@plexoos
Copy link
Member

plexoos commented Aug 13, 2021

Hi Victor, a couple questions for you:

@perevbnlgov
Copy link
Contributor Author

perevbnlgov commented Aug 13, 2021 via email

@plexoos
Copy link
Member

plexoos commented Aug 13, 2021

Victor, OnlTools is part of our repository now and it should compile as any other package in star-sw. The issue I am referring to is related to ROOT6 so you don't need to know the details about OnlTools.

As an exercise, can you please do the following exactly as typed line by line, in order to fix this problem #102 (comment)?

mkdir -p newdir
cd newdir
git clone git@github.com:star-bnl/star-sw.git
cd star-sw
git checkout -b pr/fix_onltools_root6 origin/pr/ci_build_onltools
# Open your favorite editor and
# modify the code to fix the issue with the private constructor
git add -p .
git status # optional
git commit -m "Fix issue with private copy constructor in ROOT6"
git push -u origin pr/fix_onltools_root6

Now go to pr/ci_build_onltools...pr/fix_onltools_root6 and click the "Create pull request" button. That's it.

@veprbl
Copy link
Member

veprbl commented Aug 13, 2021

If somebody explained me how to remove my first Root6 commit, I will be happy.

That's easy:

git rebase -i `git merge-base origin/main HEAD`

Then remove the line corresponding to the unfavored commit. There will be also other interesting options that you will see.

If you break something, you can always see git reflog and use git reset --hard to restore the branch to it's previous state.

@perevbnlgov
Copy link
Contributor Author

perevbnlgov commented Aug 13, 2021 via email

@plexoos
Copy link
Member

plexoos commented Aug 13, 2021

fatal: not a git repository

You need to be inside the directory with cloned repo when you issue git commands:

cd star-sw/ # or whatever the name is where you cloned
ls -a .git # then make sure the `.git` directory exists in star-sw/

@perevbnlgov
Copy link
Contributor Author

perevbnlgov commented Aug 13, 2021 via email

@veprbl
Copy link
Member

veprbl commented Aug 13, 2021

fatal: not a git repository (or any parent up to mount point /)
Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).

Do any other git commands work?

@perevbnlgov
Copy link
Contributor Author

perevbnlgov commented Aug 14, 2021 via email

@veprbl
Copy link
Member

veprbl commented Aug 14, 2021

Do any other git commands work?
Yes For instance I clone from forked.

I meant commands that work with repository: git log, git status
If those work, git rebase should work as well

@perevbnlgov
Copy link
Contributor Author

perevbnlgov commented Aug 14, 2021 via email

@plexoos
Copy link
Member

plexoos commented Aug 14, 2021

I deleted everything make a clone again

That is exactly what I suggested in #94 (comment)
Now, just create a new branch, commit changes, and share it with us by pushing it here.
Branches do not cost anything in Git. You don't want to grow the same branch for months without merging as in CVS.

@perevbnlgov
Copy link
Contributor Author

perevbnlgov commented Aug 15, 2021 via email

@veprbl
Copy link
Member

veprbl commented Aug 15, 2021

but "forked" is still the same

The remote repositories are left unchanged unless you do the git push

@plexoos
Copy link
Member

plexoos commented Aug 17, 2021

Hi Victor, any luck creating a branch with something we can merge?

mgr/ROOT_LEVEL Outdated
@@ -1 +1,2 @@
5.34.38
6.99.99
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

THis file should NOT be committed. Our root level is 5.34.38

mgr/gccfilter Outdated
@@ -0,0 +1,451 @@
#!/usr/bin/env perl
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gccfilter should not be a requirement for ROOT6. It might be nice to have, but... definitely falls under the category of "tools" not "build system". Should be located elsewhere.

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

4 participants