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 building on FreeBSD #1499

Closed
wants to merge 1 commit into from
Closed

Conversation

zaynetro
Copy link

@zaynetro zaynetro commented Feb 25, 2021

Closes owncloud/ocis#1217

Still pending:

How to test this on Linux:

export GOOS=freebsd
make

UPD: go-prompt patched c-bata/go-prompt#224 (waiting for a release)

@update-docs
Copy link

update-docs bot commented Feb 25, 2021

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@zaynetro
Copy link
Author

zaynetro commented Feb 25, 2021

Hmm,

pkg/storage/fs/ocis/ocis_unix.go:31:18: unnecessary conversion (unconvert)
    bavail := uint64(stat.Bavail) // stat.Bavail is int64 on FreeBSD

will be nice to somehow enable -all flag on unconvert linter (ref: https://github.com/mdempsky/unconvert )

Using the -all flag, unconvert will analyze the Go packages under all possible GOOS/GOARCH combinations, and only identify conversions that are unnecessary in all cases.

E.g., syscall.Timespec's Sec and Nsec fields are int64 under linux/amd64 but int32 under linux/386. An int64(ts.Sec) conversion that appears in a linux/amd64-only file will be identified as unnecessary, but it will be preserved if it occurs in a file that's compiled for both linux/amd64 and linux/386.

golang-ci uses a fork https://github.com/golangci/unconvert

@zaynetro
Copy link
Author

zaynetro commented Mar 5, 2021

OK. Now only need to figure out how to resolve unconvert lint issue golangci/golangci-lint#1809

Let me know if you have some ideas.

@zaynetro zaynetro changed the title WIP: Fix building on FreeBSD Fix building on FreeBSD Mar 5, 2021
@ishank011
Copy link
Contributor

Hi @zaynetro, thanks for the PR, the changes look good! We can wait for you to implement the changes in unconvert or golangci-lint. If that takes long to merge, we can also have separate the implementations for ocis_unix and ocis_freebsd.

@zaynetro zaynetro marked this pull request as ready for review March 20, 2021 07:03
@zaynetro zaynetro requested a review from labkode as a code owner March 20, 2021 07:03
@zaynetro
Copy link
Author

Seems like it will take a while to update golangci-lint so I split into more freebsd specific files as you suggested.

@zaynetro
Copy link
Author

@ishank011 How can we continue with these changes?

Copy link
Contributor

@ishank011 ishank011 left a comment

Choose a reason for hiding this comment

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

@zaynetro apologies for the delay. Mostly looks good. Windows syscalls do have the ENODATA error https://golang.org/src/syscall/zerrors_windows.go. Can we use that?

@@ -9,7 +9,7 @@ require (
github.com/Masterminds/sprig v2.22.0+incompatible
github.com/ReneKroon/ttlcache/v2 v2.3.0
github.com/aws/aws-sdk-go v1.37.30
github.com/c-bata/go-prompt v0.2.5
github.com/c-bata/go-prompt v0.2.6
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you revert the go-prompt upgrade? There were issues with user input with 0.2.6

Copy link
Author

Choose a reason for hiding this comment

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

go-prompt update is necessary because it fixes other build errors: c-bata/go-prompt#224.

I will take a look at Windows implementation for error handling.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah okay. Will try fixing it

@zaynetro
Copy link
Author

zaynetro commented Apr 4, 2021

zerrors_windows.go has a comment above defined contants.

// Invented values to support what package os and others expects.

Also syscall package is deprecated

Deprecated: this package is locked down. Callers should use the corresponding package in the golang.org/x/sys repository instead.

I am not sure that Windows has exact same errors as unix systems. golang.org/x/sys/windows defines completely different error types. Do you still want me to implement XattrIsNoData method on Windows using syscall.ENODATA constant? I am not familiar with file systems but to me it seems that ENODATA will never happen on Windows since there is no such thing there.

@ishank011
Copy link
Contributor

@zaynetro ah okay, we can let it be then. Can you rebase?

@zaynetro
Copy link
Author

Done.

@butonic
Copy link
Contributor

butonic commented Jul 29, 2021

@zaynetro could you rebase? I think @ishank011 would like to get this in.

Closes owncloud/ocis#1217

* Extracted common error methods to `errtypes` package
* Fixed type error on Bavail ( https://golang.org/src/syscall/ztypes_freebsd_arm64.go#L125 )
* Alias ENODATA to ENOATTR

How to test this on Linux:

```
export GOOS=freebsd
make
```
@zaynetro
Copy link
Author

Rebased.

@butonic
Copy link
Contributor

butonic commented Apr 28, 2022

@micbar you had a look at the ENOATTR ENODATA and sent an upstream PR https://github.com/pkg/xattr/pull/63/files
@C0rby don't you have a bsd variant running?

I know this PR bitrotted a bit, still @zaynetro could you retarget this PR against the edge branch? It might be easier to get it in there.

@zaynetro zaynetro requested review from a team and glpatcern as code owners June 16, 2022 11:48
@Lars-Cleemann
Copy link

Is there any update on this bug?
OCIS still does not build on FreeBSD due to this bug.

@micbar
Copy link
Member

micbar commented Dec 15, 2022

oCIS is now building on the edge branch which means it uses reva 2.x releases.

This fix would need to go to the edge branch.

@joramkruijer
Copy link

Is there any update on this bug?
OCIS still does not build on FreeBSD due to this bug.

I've just attempted to build oCIS v2.0.0 (their stable release) on FreeBSD today and can confirm this bug is still present in reva, preventing a successful compilation.. What can I do to help speed this up?

@micbar
Copy link
Member

micbar commented Jan 30, 2023

@joramkruijer please try the latest master.

@joramkruijer
Copy link

@joramkruijer please try the latest master.

Thanks, that resolves the compilation issue! There's an issue preventing ocis from running due to a bug in reva, for which I've opened a pull request (#3650).

@zaynetro zaynetro closed this Apr 2, 2024
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.

[build] OCIS not building on FreeBSD
6 participants