Netgate Discussion Forum
    • Categories
    • Recent
    • Tags
    • Popular
    • Users
    • Search
    • Register
    • Login

    Submitting PRs for packages in the pfSense/FreeBSD-ports repo?

    Scheduled Pinned Locked Moved Development
    23 Posts 7 Posters 2.4k Views
    Loading More Posts
    • Oldest to Newest
    • Newest to Oldest
    • Most Votes
    Reply
    • Reply as topic
    Log in to reply
    This topic has been deleted. Only users with topic management privileges can see it.
    • H
      heper
      last edited by

      I don't use git enough to remember how/why I get it to fuck up.

      For small changes like this, I find it much easier to use the online editor on GitHub

      1 Reply Last reply Reply Quote 0
      • luckman212L
        luckman212 LAYER 8
        last edited by

        Forgot to add – I tried that first, and GitHub gave me an error. Wish I took a screenshot of that, but TL;DR it failed.  Even if you use the online editor, it requires you to have your own fork of the project so that doesn't really get to the root of the issue.  My best guess at this point is that maybe I had an old fork of FreeBSD-ports before the "default" branch was switched to 'devel'? I do see that the 'master' branch does not contain the pfSense-pkg-xxx dirs, only devel. So maybe my fork was old and when I cloned and rebased it just sync'd to master. Who knows, I have since deleted the fork and all the branches and started w/ a clean slate.

        Still looking for an answer to question #2 – is this the "right" way to hack on pfSense?

        1 Reply Last reply Reply Quote 0
        • bmeeksB
          bmeeks
          last edited by

          FIrst question … are you trying this using the Windows Git client or are you using a Linux workstation?  I ran into the exact same problem with the Windows Git client.  It just does not like the FreeBSD-ports repository for some reason (or at least it never worked for me).  I instead use a CLI based version of Git on a FreeBSD virtual machine and everything works fine.

          You should submit pull requests against the DEVEL repository for FreeBSD-ports.  The pfSense guys will do the merging to the pfSense release branch.

          Bill

          1 Reply Last reply Reply Quote 0
          • luckman212L
            luckman212 LAYER 8
            last edited by

            Using macOS - git 2.14.2 from Homebrew

            1 Reply Last reply Reply Quote 0
            • bmeeksB
              bmeeks
              last edited by

              Since Mac is an OpenBSD derivative, I would think it should work but maybe not.  If you have access to a Linux machine or a FreeBSD virtual machine, try that using the CLI version of Git.  One issue may be the huge number of directories and files in the repository.  That's what appeared to be tripping up Windows.

              Bill

              1 Reply Last reply Reply Quote 0
              • luckman212L
                luckman212 LAYER 8
                last edited by

                Git didn't throw any errors when cloning (the error happened on GitHub's site actually). But I'll spin up a VM and do some more testing. Thanks

                1 Reply Last reply Reply Quote 0
                • bmeeksB
                  bmeeks
                  last edited by

                  @luckman212:

                  I'm a relative beginner at git/GitHub and still learning new things every day. So forgive my ignorance on this!

                  I was trying to submit a small patch for the HAProxy package today and ran into some trouble. I worked around it but am not sure if I've done so correctly. My typical workflow when working on patches to the main pfsense/pfsense repo has been:

                  1. fork the main repo on GitHub
                  2. clone my fork of the repo
                  3. hack hack hack
                  4. commit + push

                  When I tried that on the pfsense/FreeBSD-ports repo, I ended up with a clone that was missing all of the files I needed. Specifically, all of the pfSense-pkg-xxxx directories which contained the files I was trying to update were just not there. What I ended up doing instead was cloning the original repo, hacking on that, and then pushing the commits, like this:

                  git clone git@github.com:pfsense/FreeBSD-ports.git my-foobar-patch
                  cd my-foobar-patch
                  git remote set-url origin git@github.com:luckman212/FreeBSD-ports.git
                  git checkout -b my-foobar-patch
                  
                  ~ HACK HACK HACK ~
                  
                  git ls-files --modified | xargs git add
                  git commit
                  git push origin my-foobar-patch
                  

                  That worked.  So, questions are:

                  1. why did that happen?
                  2. have I done this the "right" way?

                  Thanks all

                  As for question #2 –

                  What you have is fine with a couple of tweaks as follows.  First, I usually just use "git commit -a" instead of searching for modified files manually.  Git knows what files have been touched and the "-a"  switch will automatically include them in the commit.

                  The bigger tweak you need is to create a new branch each time you start a patch stream.  For example, when I am getting ready to create an update for the Snort package I do the following --

                  
                  git pull upstream devel
                  git checkout -b pfSense-pkg-snort-3.2.9.5_2
                  
                  

                  This first pulls any updates from the main FreeBSD-ports repository into my local copy.  The next step creates a new branch in my local copy to match the package name and package version I will be submitting changes against.

                  When I'm ready to commit my updates I do this:

                  
                  git commit -a
                  git push origin pfSense-pkg-snort-3.2.9.5_2
                  
                  

                  These two lines will commit all my modified files and then push them as a new branch to my Git hub fork of the FreeBSD-ports repository.  I then go to the Git Hub web site and my new branch will show up in remote repository along with a button to create a Pull Request against the upstream FreeBSD-ports repository of pfSense.

                  The above examples assume pfSense-pkg-snort-3.2.9.5_1 is the current devel version and pfSense-pkg-snort-3.2.9.5_2 is my updated package to be submitted as a Pull Request.

                  Bill

                  1 Reply Last reply Reply Quote 0
                  • luckman212L
                    luckman212 LAYER 8
                    last edited by

                    Thanks for your tips - very helpful.  :)

                    1 Reply Last reply Reply Quote 0
                    • bmeeksB
                      bmeeks
                      last edited by

                      @luckman212:

                      Thanks for your tips - very helpful.  :)

                      Another helpful tip that benefits the pfSense guys during code review is to break your package changes into a series of commits instead of one giant commit.  For instance, let's say I'm fixing four bugs in the Snort GUI code and I plan to include them all in the same update.  I will fix bug #1, then do a commit so that only the files touched while fixing bug #1 get marked.  Next I fix bug #2 and commit again.  Rinse and repeat for the other two bugs.  At the end I have a chain of four commits for bug fixes.  I will then do a final commit that includes only the file changes needed to bump the package version info (typically just one or two edited lines in the Makefile).  So the entire package update would consist of five separate commits into the same branch.

                      Now as folks review the changes, everything is clearly laid out and easy to follow with one commit per change.  Take a look at some of the closed pull requests on the Git Hub site for the pfSense/FreeBSD-ports repository for examples.

                      Bill

                      1 Reply Last reply Reply Quote 0
                      • jimpJ
                        jimp Rebel Alliance Developer Netgate
                        last edited by

                        @bmeeks:

                        Another helpful tip that benefits the pfSense guys during code review is to break your package changes into a series of commits instead of one giant commit.

                        While that is nice for review, it makes it more difficult for us to cherry-pick that change to other branches, such as copying the commit(s) from devel to RELENG_2_4_0, RELENG_2_3, and RELENG_2_3_4.

                        It's nice to have them all squashed into a single commit before it's merged, but we have some scripts that help us deal with that so it's not the end of the world.

                        Remember: Upvote with the 👍 button for any user/post you find to be helpful, informative, or deserving of recognition!

                        Need help fast? Netgate Global Support!

                        Do not Chat/PM for help!

                        1 Reply Last reply Reply Quote 0
                        • luckman212L
                          luckman212 LAYER 8
                          last edited by

                          Thanks jimp, so - single unified commits are preferred for PRs then?

                          1 Reply Last reply Reply Quote 0
                          • jimpJ
                            jimp Rebel Alliance Developer Netgate
                            last edited by

                            Since we don't merge on github it's a little more complicated, but IIRC you can squash the commits in your PR after review so you can have multiple commits while you work, and then it can be merged as one.

                            https://github.com/blog/2141-squash-your-commits
                            http://blog.steveklabnik.com/posts/2012-11-08-how-to-squash-commits-in-a-github-pull-request

                            Remember: Upvote with the 👍 button for any user/post you find to be helpful, informative, or deserving of recognition!

                            Need help fast? Netgate Global Support!

                            Do not Chat/PM for help!

                            1 Reply Last reply Reply Quote 0
                            • D
                              doktornotor Banned
                              last edited by

                              @jimp:

                              but IIRC you can squash the commits in your PR after review

                              That's what you would need to activate, none of the people submitting PRs can use that. It can only be used on merge. You have kind of an issue with the workflow you are using.

                              1 Reply Last reply Reply Quote 0
                              • jimpJ
                                jimp Rebel Alliance Developer Netgate
                                last edited by

                                @doktornotor:

                                @jimp:

                                but IIRC you can squash the commits in your PR after review

                                That's what you would need to activate, none of the people submitting PRs can use that. It can only be used on merge. You have kind of an issue with the workflow you are using.

                                When I'm working locally, I can squash several commits down to one. I think the same thing can be done in a PR by the submitter, but I'm not 100% on that. I know for certain it can be done before submitting the PR. There are lots of how-tos out there on squashing, such as https://github.com/todotxt/todo.txt-android/wiki/Squash-All-Commits-Related-to-a-Single-Issue-into-a-Single-Commit – Yes, github can do that automatically when doing a merge in their GUI, but that doesn't help us.

                                The workflow has to be the way it is because everything has to be sourced from our local gitlab instance. Aside from it being a better flow for us internally, we don't like being stopped dead in our tracks if github goes down, which happens a lot. Some people don't notice how often it happens, but when you're working in git all day, every day, it's more apparent.

                                There may also be something we can cook up eventually with local scripts to squash but I'm not sure any of us have looked at it.

                                Remember: Upvote with the 👍 button for any user/post you find to be helpful, informative, or deserving of recognition!

                                Need help fast? Netgate Global Support!

                                Do not Chat/PM for help!

                                1 Reply Last reply Reply Quote 0
                                • D
                                  doktornotor Banned
                                  last edited by

                                  Well yes you can squash things locally… what you linked regarding GitHub squash feature however is unusable for anyone without merge (and apparently for people with merge privs as well due to the workflow).

                                  1 Reply Last reply Reply Quote 0
                                  • jimpJ
                                    jimp Rebel Alliance Developer Netgate
                                    last edited by

                                    Yeah but the question is if you can squash to your own PR after creating the PR, before it gets merged. I'll have to try to test that out.

                                    Remember: Upvote with the 👍 button for any user/post you find to be helpful, informative, or deserving of recognition!

                                    Need help fast? Netgate Global Support!

                                    Do not Chat/PM for help!

                                    1 Reply Last reply Reply Quote 0
                                    • bmeeksB
                                      bmeeks
                                      last edited by

                                      @jimp:

                                      @bmeeks:

                                      Another helpful tip that benefits the pfSense guys during code review is to break your package changes into a series of commits instead of one giant commit.

                                      While that is nice for review, it makes it more difficult for us to cherry-pick that change to other branches, such as copying the commit(s) from devel to RELENG_2_4_0, RELENG_2_3, and RELENG_2_3_4.

                                      It's nice to have them all squashed into a single commit before it's merged, but we have some scripts that help us deal with that so it's not the end of the world.

                                      Didn't realize it complicated the cherry picking process.  I'm not a GitHub guru, though.  Based on the conversations that followed in this thread, it looks like there are pros and cons each way (multiple commits versus single giant commit).

                                      Bill

                                      1 Reply Last reply Reply Quote 0
                                      • D
                                        doktornotor Banned
                                        last edited by

                                        Those giant commits definitely suck when searching for regressions. Plus mostly unusable with GitHub web interface, it just won't show anything lots of times, instead complaining about the query taking too much time etc.

                                        1 Reply Last reply Reply Quote 0
                                        • ?
                                          Guest
                                          last edited by

                                          @jimp:

                                          Yeah but the question is if you can squash to your own PR after creating the PR, before it gets merged. I'll have to try to test that out.

                                          Yes you can, that's what I do now after initially having a load of commits for typos and additions in my earlier PR's. Now I do a git rebase -i HEAD~x locally, where x is the number of commits to squash and then a git push origin whatever-im-doing –force

                                          That syncs it all up nicely. I'm 99.9% certain the PR still points at the branch if I've issued a PR from, that's what the a quick Google of Git says too. My testers end up with one nice patch ID and you end up with a single commit.

                                          1 Reply Last reply Reply Quote 0
                                          • L
                                            loonylion
                                            last edited by

                                            @bmeeks:

                                            …

                                            I ran into the exact same problem with the Windows Git client.  It just does not like the FreeBSD-ports repository for some reason (or at least it never worked for me).

                                            ...

                                            Bill

                                            It doesn't work because there's files in the repo with names reserved by Windows (specifically 'LPR' IIRC) I think its probably a holdover from the DOS days but maybe they're still used internally in Windows.

                                            1 Reply Last reply Reply Quote 0
                                            • First post
                                              Last post
                                            Copyright 2025 Rubicon Communications LLC (Netgate). All rights reserved.