RfidResearchGroup / proxmark3

Iceman Fork - Proxmark3
http://www.icedev.se
GNU General Public License v3.0
4.04k stars 1.06k forks source link

Consider recreating repo as a fork of iceman1001/proxmark3 #18

Closed gkelly closed 6 years ago

gkelly commented 6 years ago

Since this repo wasn't created via the "fork" button on the iceman1001 (or even the proxmark/proxmark3 repo) you can't easily create a PR for the upstream or iceman1001 repo and this repo with just a click. It's a minor thing, but it means that the first time someone wants to contribute a patch to iceman1001 and this repo that they need to fork both repos and create a PR in both of their forks.

Since the commit hashes will be the same you should be able to delete this repo, fork iceman1001 here again, and then git push -f the correct state back in with nobody being the wiser.

gkelly commented 6 years ago

Yes, you'd lose the other 4 issues that are open on this repo. But I feel like now's the time. :smile:

RfidResearchGroup commented 6 years ago

I hear what you are saying. Already dropped it once and recreated this one. The different repos is good to separate it from iceman fork, since its not focused on rdv40. I am reluctant to this idea, but I appreciate your feedback.

gkelly commented 6 years ago

Sorry, I think I didn't explain my initial idea very well. I think having a separate repo for rdv4-centric work is a good idea. What I mean is:

  1. Make sure you have an up-to-date clone of this repo.
  2. Delete this repo.
  3. Go to iceman1001/proxmark3 and click Fork.
  4. On your local clone, git push -f.
  5. Now this repo exists again (but the issues will be gone).

That way you'll end up with a github repo that knows it's a fork of iceman1001/proxmark3. The benefit here is that when someone contributes a PR to both projects that github will then know to show this fork in the project selector for creating the PR.

CumpsD commented 6 years ago

Would probably make sharing easier too

JamesRHarris commented 6 years ago

I second this. Being an owner of both the RDV3 and RDV4 I would prefer to have one set of tools that can support both. While both may have different development branches at some point merging stable features/support back would be great.

JamesRHarris commented 6 years ago

From the readme

Why didn't you based it on offical PM3 Master? The separation from offical pm3 repo gives us very much freedom to create a firmware/client that suits the RDV40 features. We don't want to mess up the offical pm3 repo with RDV40 specific code.

This is completely false. First your branch can have any number of updates that are ahead of an upstream master. At no point must pull requests happen all a proper fork does is help facilitate pull should they be accepted.

There are hundreds of ways to add in RDV40 code that wouldn't pollute upstream and other builds. Only lazy coding which doesn't properly use layering and separation would produce this problem. Enabling the RDV40 code could be done at build time (see configure) or it could happen at run time on device detection.

This explanation boils down to we are too lazy or don't know how to play nice with upstream. Neither of which support the decision not to be a proper fork. Pulls would just be delayed until this project got its crap together.

gkelly commented 6 years ago

I think I might have done an exceedingly poor job of explaining the issue. This issue isn't about whether there should be a forked repo. I taking it as a foregone conclusion that there will be a fork repo. I just want the GitHub metadata to point to the iceman repo as the repo which this was forked from so I can one-click Pull Request into this repo. :grimacing:

JamesRHarris commented 6 years ago

I think I might have done an exceedingly poor job of explaining the issue. This issue isn't about whether there should be a forked repo. I taking it as a foregone conclusion that there will be a fork repo. I just want the GitHub metadata to point to the iceman repo as the repo which this was forked from so I can one-click Pull Request into this repo. 😬

Not doing this as you pointed out will just set this repo to be underutilized. Someone will properly fork upstream then manually merge the changes from here. As that repo will properly track upstream and allow for pulls it will likely get more community support. If RFIDResearchGroup can't understand soon it would almost be better to make that happen now before this dead end repo diverges too much making the manual patching more difficult.

TomHarkness commented 6 years ago

Hmm interesting thoughts guys. Many good opinions here. Just how to move forward? Maybe we need to do some kind of community poll to get some insights in to how the majority feel about this situation?

BreakSecurity commented 6 years ago

I agree please consider recreating repo as a fork of iceman1001/proxmark3

merlokk commented 6 years ago

Maybe its too late.

RfidResearchGroup commented 6 years ago

closed.

JamesRHarris commented 6 years ago

It seems the RFIdResearchGroup isn't going to do the right thing. So Before we get too far down the path who wants to properly fork iceman then rebase the changes in this repo onto that proper fork ? @TomHarkness @gkelly @BreakSecurity @CumpsD any takers ??

BreakSecurity commented 6 years ago

I can not take care of it, sorry

TomHarkness commented 6 years ago

Look, I am really unsure how exactly this should move forward as I am not mainly a software dev, I focus a lot on hardware and add to the software repo where I cam, as I learn more. Problem is that this repo now has SO many changes that are rdv4 specific and would take a huge amount of time and effort to merge into the master repo.

JamesRHarris commented 6 years ago

As I doubt they have taken many (if any) changes from up stream. It shouldn't be all that hard for someone with experience with git. Basically we need to fork upstream at the point where this repo started. Then apply the change history (rebase) these changes on top and done. It would be much more of a mess should there have been upstream patches here and there but I doubt we will find any.

On Sun, Nov 18, 2018 at 1:27 PM Tom Harkness notifications@github.com wrote:

Look, I am really unsure how exactly this should move forward as I am not mainly a software dev, I focus a lot on hardware and add to the software repo where I cam, as I learn more. Problem is that this repo now has SO many changes that are rdv4 specific and would take a huge amount of time and effort to merge into the master repo.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/RfidResearchGroup/proxmark3/issues/18#issuecomment-439727276, or mute the thread https://github.com/notifications/unsubscribe-auth/ANFl_3_5u2v82Cw7y8AzMunq6SN5odHIks5uwdDPgaJpZM4WKCap .

-- James Harris Software Engineer james.r.harris@gmail.com

merlokk commented 6 years ago

it depends... if it needs to sync with official proxmark. it almost impossible. it like 2000 commits ahead/behind. but if it needs to have fork of Iceman's fork. it possible, but it have like 60k-100k lines that needs merge. it not impossible... but it very long work...

JamesRHarris commented 6 years ago

@merlokk I disagree. This is a common problem that is solved all the time. First find the proper branch point of icemans repo. Then apply all the changes from this repo, which should go into without a problem. Since we are mostly just fixing up the meta data the state of the files should be the same as what is currently here. Then this 'issue' is closed. Now if we want to todo a pull request back into icemans upstream that typically is done via a rebase. That's where we need to sort out the changes since this branch and the current tip of iceman, but that is a separate task for another day. The point is we still have it as an option. Similarly once we have the upstream fixed we can pull from upstream to get the improvements that are going into iceman. Setting up the upstream correctly with a proper fork does enables us not only to pull these changes into iceman but also allows us to pull icemans fixes here.

BreakSecurity commented 5 years ago

Any news on this @TomHarkness