Page MenuHomeFreeBSD

Port for net/samba412
ClosedPublic

Authored by se on Sep 20 2020, 9:50 AM.

Details

Reviewers
timur
Summary

I have created a new port for net/samba412, based on samba411.

The port has only been tested with a few combinations of options, but since the changes relative to the previous version are not that big, I assume that it will work for most settings.
It contains empty patch files as markers for patches that are in net/samba411 but are no longer required in net/samba412.
I have not checked whether the man-pages in files/man are still fully applicable to this version.

Test Plan

Build and run with different options on different architectures and FreeBSD releases.

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

se requested review of this revision.Sep 20 2020, 9:50 AM

Apparently iXsystems maintain their own version which they might want to maintain upstream?
https://github.com/freenas/ports/tree/truenas/12.0-stable/net/samba
They do seem to carry quite a bit of patches that might/might not be useful.
https://github.com/freenas/ports/blob/freenas/master/net/samba/Makefile (4.13.0rc2)

Apparently iXsystems maintain their own version which they might want to maintain upstream?

As a former iXsystems employee I don't think there is much interest from their side in supporting ports version.

At one point we tried to merge the efforts and have universal port that would fit both projects, but it didn't fly.

Andrew does a great job and has way bigger expertise of the Windows side of the CIFS, but I can imagine he is utterly busy with the iX version of the port to carry an additional burden.

Saying all this, I'd also would like to remind that iX is switching to Linux in its future versions, so, again, that wouldn't fly.

Hi, Stefan!

Thank you for your submission, but I have to admit it's a huge one and really, can't be observed easily.

As you say you based it on the samba411, can you, please, provide something like diff -runN between those versions? At the moment it's hard to say what you've actually change besides the version number and what is added and what is dropped/modufied.

Regards,
Timur

I had preferred to upload this new port in a way that allows to compare it with net/samba411, but I was afraid that the automatisms when closing the review might lead to confusing results.
If there is an easy way to submit this patch (destined for a new port) on top of an existing patch, I'd like to learn how to do it ...

I have created a diff relative to net/samba411 and have made it available from: https://people.freebsd.org/~se/ports/samba412-upgrade.diff

There are some patches that have been deleted, one chunk has been moved to a new source file, the Makefile is not complete (the CONFLICTS line is commented out but has to be adjusted before the commit (in all the Samba-Ports), anyway).

Thanks, that at least is readable :)

Few questions regarding the patch:

I remember seeing in the past that those were missing, but not with recent enough versions of 4.12. Are you sure those dependencies are necessary?

+# ICU
+LIB_DEPENDS+= libicuuc.so:devel/icu
+# unwind
+LIB_DEPENDS+= libunwind.so:devel/libunwind

Thanks for the hint with the:

+GLUSTERFS_CFLAGS+= -DHAVE_GFAPI_VER_7_6

That trigged me to fix net/glusterfs to provide proper API version :)

You, seems, touched provisioning patch, but, seems, by just brut forcing it :)

One change I don't get is:

+@@ -2340,6 +2350,9 @@
+

if not os.path.isdir(paths.netlogon):
    os.makedirs(paths.netlogon, 0o755)

++
+ if smbd.have_nfsv4_acls() and smbd.has_nfsv4_acls(paths.sysvol):
+ smbd.set_nfsv4_defaults()
-+
+

if adminpass is None:
    adminpass = samba.generate_random_password(12, 32)
  • adminpass_generated = True

Why to remove this? ^^^

I have added the explicit dependencies on ICU and UNWIND due to stage-qa output.
They may be required and included by dependencies, not Samba itself - I did not check that and since I had both already installed would have noticed if they had been found by accident only.
A poduriere build should give an answer whether the explicit dependencies are required (well, probably not, if you removed them before).
I did not test build in poudriere since too many dependencies needed to be upgraded first and I wanted to spend my limited time on getting the other issues resolved ...

The GLUSERFS version was required due to a few changed function signatures. It was obvious in the sources that this exact macro needed to be defined and I did not look any further, since I just wanted to get the port built.

Regarding the "one change you do not get": No, these changes were unintentional. I worked through the existing patch files trying to understand what still applied and just did not notice this accidental removal.

But that's why I did not want to commit this update, anyway, just get you quick-started, since Samba-4.12 has been out for a while and there was a question about its planned availability in a mail list.

I'm not sure about the state and applicability of the man-pages provided with the port - some might need to be adapted to the new version.

The pkg-plist seems to have not been sorted in ASCII order - I had to move around a lot of entries that contained underscores to get the minimal diff.
And I seem to have missed one file: %%PYTHON_SITELIBDIR%%/samba/tests/samba_tool/user_virtualCryptSHA.py in the second last chunk of the diff already existed and is only moved up by a few lines.
I had preferred if the local order (within sections) was according to ASCII - that had spared me quite some time spent on moving filenames to reduce the size of the diff.

In D26491#589849, @se wrote:

I have added the explicit dependencies on ICU and UNWIND due to stage-qa output.
They may be required and included by dependencies, not Samba itself - I did not check that and since I had both already installed would have noticed if they had been found by accident only.
A poduriere build should give an answer whether the explicit dependencies are required (well, probably not, if you removed them before).
I did not test build in poudriere since too many dependencies needed to be upgraded first and I wanted to spend my limited time on getting the other issues resolved ...

As I said, I had this output for 4.12.3 or so, but don't see it anymore for 4.12.[5-7]. Not sure, if that is fixed or my environment is tainted somehow.

The GLUSERFS version was required due to a few changed function signatures. It was obvious in the sources that this exact macro needed to be defined and I did not look any further, since I just wanted to get the port built.

Ok, I hope I fixed it properly, so when accepted, we wouldn't need extra hacking there.

Regarding the "one change you do not get": No, these changes were unintentional. I worked through the existing patch files trying to understand what still applied and just did not notice this accidental removal.

Actually, this whole patch is the biggest blocker. You missed change of the signatures and code there and I'm still struggling with it. Need a really proper patch to be submitted to upstream, so we don't carry it on.

But that's why I did not want to commit this update, anyway, just get you quick-started, since Samba-4.12 has been out for a while and there was a question about its planned availability in a mail list.

Right. Lack of time plus a lot of not obvious staff in the huge pule of patches in the port...

I'm not sure about the state and applicability of the man-pages provided with the port - some might need to be adapted to the new version.

This has to be regenerated and copied over, no problem. The whole idea is not to bring dockbook as a huge dependency while we have (most) of the manpages already there.

The pkg-plist seems to have not been sorted in ASCII order - I had to move around a lot of entries that contained underscores to get the minimal diff.

And I seem to have missed one file: %%PYTHON_SITELIBDIR%%/samba/tests/samba_tool/user_virtualCryptSHA.py in the second last chunk of the diff already existed and is only moved up by a few lines.

That's OK, I have my own plist anyhow :)

I had preferred if the local order (within sections) was according to ASCII - that had spared me quite some time spent on moving filenames to reduce the size of the diff.

I also try to keep them in sorted order, but seems my sort has different rules in regards to underscores... My locale is en_US.UTF-8.

Maybe we should create a GitHub repository for this update (initialize with the net/samba411 port and apply the patch or copy the new port over it). This would allow to co-ordinate further work. Or do you already have it available i some public repo?

If you have already improved what I submitted, I'd like to have access to perform some tests.

The version uploaded in this patch works for me in general, but I have just noticed that TimeMachine backups from my MacBook do no longer work. I have build with the FRUIT option, need to investigate what's failing ...

In D26491#590242, @se wrote:

Maybe we should create a GitHub repository for this update (initialize with the net/samba411 port and apply the patch or copy the new port over it). This would allow to co-ordinate further work. Or do you already have it available i some public repo?

You can join project at https://gitlab.com/samba-freebsd

But it haven't been touched for a long time, mostly cause I didn't find the way how to integrate ports and GitLab CI

If you have already improved what I submitted, I'd like to have access to perform some tests.

Well, it was more like I've taken some snippets from your diff to apply to my development tree, but whatever.. I wanted just to push it to the ports tree, but if you want I can place archive somewhere for testing first.

The version uploaded in this patch works for me in general, but I have just noticed that TimeMachine backups from my MacBook do no longer work. I have build with the FRUIT option, need to investigate what's failing ...

That would be a great pity. Well, I've seen you altered some vfs_fruit module code, so hopefully the problem is there... But in general this is one of the most fragile modules in Samba.

I'm accepting this revision, although the resulting samba412 is coming from my own development tree with some additions from this request. I hope, nothing was lost in such a merge, but if you spot that something is missing - please, let me know!

With best regards,
Timur

This revision is now accepted and ready to land.Sep 27 2020, 10:23 PM