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

Create redistributable package for Julia #150

Closed
Andrey1994 opened this issue Dec 1, 2020 · 50 comments
Closed

Create redistributable package for Julia #150

Andrey1994 opened this issue Dec 1, 2020 · 50 comments
Labels
chore Minor improvements, refactoring, etc enhancement New feature or request good first issue Good for newcomers

Comments

@Andrey1994
Copy link
Member

Andrey1994 commented Dec 1, 2020

Not sure that its possible to bundle native libraries with Julia packages right now, but need to double check and investigate different options

@Andrey1994 Andrey1994 added chore Minor improvements, refactoring, etc enhancement New feature or request good first issue Good for newcomers labels Dec 1, 2020
@matthijscox
Copy link
Member

matthijscox commented Dec 1, 2020

As mentioned, we can examine BinaryBuilder.jl . I'll see if I can do that myself. But perhaps you prefer to control the build process yourself?

@Andrey1994
Copy link
Member Author

Andrey1994 commented Dec 1, 2020

Here its not a few cpp files which you can easily compile wo modern C++ build systems.

I use Cmake to build it, and config file for Cmake is pretty big(https://github.com/brainflow-dev/brainflow/blob/master/CMakeLists.txt) I would like to avoid porting it if its possible.

Best case scenario and how it works for all other languages - compile C++ code using C++ specific build system(Cmake in our case) and add compiled libraries to bindings. If it's possible for Julia - its the best option, if not - need to check alternatives.

E.g how it looks in python - you build c++ code first and when you build python package libraries are included into whl via package data https://github.com/brainflow-dev/brainflow/blob/master/python-package/setup.py#L25 So this way python users dont need to worry about compilation at all and can download package with precompiled libs included

@matthijscox
Copy link
Member

matthijscox commented Dec 2, 2020

OK, so for Python the PYPI installs the binaries automatically in /lib. That's nice indeed, I haven't yet seen that for Julia.

For future reference:
I'll start by investigating Yggdrasil which is the backbone of BinaryBuilder.jl. It links to PRs of example packages implementing the Artifact system, like Cairo.jl 293

And this blog post is mentioned.

As mentioned in the blog "the artifacts subsystem is much more general and is widely applicable to all Julia packages." This suggests there must be a way to deploy a manual artifact via the package manager. The question is how difficult it is for a newbie like me.

Also the docs have a section on Artifacts:
"These containers, (called "Artifacts") can be created locally, hosted anywhere, and automatically downloaded and unpacked upon installation of your Julia package."

Come on, it must be possible!

@Andrey1994
Copy link
Member Author

Andrey1994 commented Dec 2, 2020

" so for Python the PYPI installs the binaries automatically in /lib"
Not 100% correct, PYPI doesnt install it to /lib. Its distributed as a ZIP archive in fact and you can add any files to this archive, I am adding them manually to /lib subfolder inside this arch. And in Python binding I load these libs from archive by full path

This https://julialang.github.io/Pkg.jl/dev/artifacts/ looks promising, I will read about it too

@matthijscox
Copy link
Member

matthijscox commented Dec 2, 2020

yeah, I see. The Julia _jll packages are also distributed as tarballs, so it just unpacks the folder structure you defined yourself in the tar ball.

The Julia packages typically split the package in 2; one the regular Julia code, the other the _jll wrapper and libraries. But it's not necessary.

Apparently all we need is an Artifacts.toml file which we fill with the relevant information:

[[brainflow]]
arch = "x86_64
git-tree-sha1 = "c7c61446219c91ab68a5841186d59b2dba00ab2c"
libc = "glibc"
os = "linux"

    [[brainflow.download]]
    sha256 = "... dunno yet what this, maybe you do..."
    url = "https://github.com/brainflow-dev/brainflow/.... wherever you have your linux tar ball..."

Then we should be able to add your package, use it and the library should be installed. We can try with:

import Pkg
Pkg.add(url="https://github.com/brainflow-dev/brainflow.git", subdir="julia-package/brainflow")
using brainflow

Note Julia package are typically uppercase camelcase, so I expected BrainFlow

@matthijscox
Copy link
Member

Ah wait, I'm checking out the release page. You don't have cross-compiled binaries available right? So we still need the client to compile somehow. Or we do have to set up something like BinaryBuilder.jl for your package.

@Andrey1994
Copy link
Member Author

Andrey1994 commented Dec 2, 2020

There are libs for all OSes bundled inside whl\jar\nuget files. We can add compiled C++ libs as well to release page, or upload it somewhere else if needed.

For example currently CI uploads all artifacts to AWS S3 maybe we can use it instead github releases

@matthijscox
Copy link
Member

matthijscox commented Dec 2, 2020

Ok, then I guess the easiest way would be to just have the /lib folder in a .tar.gz or something. Do you have that available behind a url somewhere?

I can also try to see what happens if I use the .whl url.

@Andrey1994
Copy link
Member Author

I can patch this https://github.com/brainflow-dev/brainflow/blob/master/.github/workflows/run_unix.yml#L213 for all OSes and configure AWS to provide download link...

I dont think that targz is a good option since for Julia it should and can work for all OSes, I doubt that it will open targz file on windows automatically. Can it download multiple files or only one?

@matthijscox
Copy link
Member

matthijscox commented Dec 2, 2020

Just tried the docs example. I got an error.. but in the end it did work...
And I can unpack the .tar.gz on windows
Anyway Julia uses a Tar.jl package, so it's all included in the language itself.

image

@Andrey1994
Copy link
Member Author

Good to know, I think then we can create zip\targz with libs and upload it to release page

@matthijscox
Copy link
Member

matthijscox commented Dec 2, 2020

TODO: check if tar.gz is really necessary

TODO: check create_artifact(), archive_artifact(), bind_artifact!(), as used here for example: https://github.com/r3tex/ObjectDetector.jl/blob/master/dev/artifacts/generate_artifacts.jl

Also https://github.com/CiaranOMara/ArtifactHelpers.jl

@matthijscox
Copy link
Member

Ok I think I figured it out, at least inside Julia:

using Pkg
using Pkg.Artifacts
using SHA

# create unpacked hash
unpacked_artifact_dir = "path_to_brainflow/lib"
# hash_sha1 = Base.SHA1(Pkg.GitTools.tree_hash(unpacked_artifact_dir)) # manual approach
hash_sha1 = create_artifact() do artifact_dir
    cp(unpacked_artifact_dir, artifact_dir, force=true)
end

# TODO: package tar ball
# hash_sha256 = bytes2hex(open(sha256, filename))
# or do it automatically:
tarball_filepath = joinpath(mktempdir(), "brainflow.tar.gz")
tarball_hash = archive_artifact(hash_sha1, tarball_filepath)

artifact_toml = joinpath(@__DIR__, "Artifacts.toml")
name = "brainflow"
tarball_url = "https://somelink/brainflow-vx.tar.gz" # TODO: upload the tarball
bind_artifact!(artifact_toml, name, hash_sha1; download_info=[(tarball_url, tarball_hash)], force=true)

This added the following to my local Artifacts.toml :

[brainflow]
git-tree-sha1 = "e3a5b1ec632bc824dcd43b27bb6ace2ffe323465"

    [[brainflow.download]]
    sha256 = "3cbbf892209cac5ca346efc446a61fd5b076278b165773e5e7bd727ce8a3ac50"
    url = "https://somelink/brainflow-vx.tar.gz"

Now when I activate my environment in this directory with the Artifacts.toml I can do:

julia> artifact"brainflow"
"C:\\Users\\matcox\\.julia\\artifacts\\e3a5b1ec632bc824dcd43b27bb6ace2ffe323465"

When I browse this folder it contains the unpacked /lib files

@Andrey1994
Copy link
Member Author

julia> artifact"brainflow"
"C:\\Users\\matcox\\.julia\\artifacts\\e3a5b1ec632bc824dcd43b27bb6ace2ffe323465"

Its good, more likely will need to use this path in Julia code to load dlls by full path or set PATH\LD_LIBRARY_PATH to load libs

@matthijscox
Copy link
Member

Not sure I understand, but we can just use this path to initialize the DLL paths:

using Pkg.Artifacts

const INTERFACE_PATH = artifact"brainflow"

function interface_path(library::AbstractString)
    return abspath(INTERFACE_PATH, interface_name(library))
end

function interface_name(library::AbstractString)
    if Sys.iswindows()
        return "$library.dll"
    elseif Sys.isapple()
        return "lib$library.dylib"
    else
        return "lib$library.so"
    end
end

const DATA_HANDLER_INTERFACE = interface_path("DataHandler")
const BOARD_CONTROLLER_INTERFACE = interface_path("BoardController")
const ML_MODULE_INTERFACE = interface_path("MLModule")

@Andrey1994
Copy link
Member Author

Andrey1994 commented Dec 4, 2020

One question about downloaded artefacts.. will it still be possible to use local libraries if I have them?
Or in case if I am making changes in C++ code and need to make corresponded change in Julia code.

I can copypaste libs locally to required location for such debugging the question is will Julia fail if there is no file which it tries to download or not and is it possible to disable downloading

@Andrey1994
Copy link
Member Author

I added compiled libs to release page, and its done automatically by CI now.
Here is a link for latest release https://github.com/brainflow-dev/brainflow/releases/download/3.7.2/compiled_libs.zip
Its zip instead targz and there is no lib folder inside this arch

@matthijscox
Copy link
Member

matthijscox commented Dec 4, 2020

I asked, and for automatic unpacking upon downloading of the artifact only tarballs are supported: https://github.com/JuliaLang/Pkg.jl/blob/353a7cb4c46b1c78dff6755c92071498b86deec9/src/PlatformEngines.jl#L463

So no .zip support. If we want to use .zip artifacts we will have to still write an unzipping function in our package __init__() or something.

For debugging, if the artifact is already downloaded I believe it doesn't download it again, so you could find its location and overwrite the content, but I havent tried. Also the point of using artifacts is that you don't care about their absolute location.

But do you really consider this an important use-case, where you tightly prototype both the c++ code and a binding at the same time? (I'd rather prefer to look for automated binding generation from the header files or something)

@Andrey1994
Copy link
Member Author

<< So no .zip support. If we want to use .zip artifacts we will have to still write an unzipping function in our package init() or something.

Ok, I will change to targz

<< But do you really consider this an important use-case, where you tightly prototype both the c++ code and a binding at the same time?

Yes, and its important. For example when you are adding new methods to the C++ part you need to add them to the Julia part as well and as of right now its done manually. Even if you do it automatically before publishing new release or pushing to the repo you need to test this change and you need to use local libraries for it

@Andrey1994
Copy link
Member Author

Andrey1994 commented Dec 4, 2020

Parsing headers automatically is a terrible idea in practice, matlab is trying to do it and I had huge amount of problems with it. There are dirty hacks in brainflow code to fix it in matlab.

Matlab uses regexps and these regexps do not work at all for non trivial function declarations.
fozqgb59a9p21

In other languages(python,java,c#,julia,etc) its done manually and its much more relaible.

@matthijscox
Copy link
Member

Alright I'll have a look. I'm sure we can change the library locally, worst case we temporarily change the library path while testing.

@matthijscox
Copy link
Member

matthijscox commented Dec 9, 2020

Once artifacts work: publish the Julia package. (or make this a separate issue?)

Andrey mentioned publishing/deployment is now done here: https://github.com/brainflow-dev/brainflow/blob/master/.github/workflows/deploy.yml#L149

Changes to be made for Julia brainflow package:

  • Project.toml version update (todo in CI)
  • Artifact usage and Artifact.toml update (todo in CI)
  • Register with JuliaRegistrator. Can be made automatic in CI?
  • update Julia brainflow installation documentation
  • other things?

Question: change package name from brainflow to BrainFlow?

@Andrey1994
Copy link
Member Author

I think artifacts and package can be in the same task\issue

Version update for other languages done in CI by string replace, here it can be the same
About subdir... I faced this issue before and didnt find a solution for it

I dont think that we need to rename package, idk about julia but in pypi for example package names are not case sensetive

@matthijscox
Copy link
Member

matthijscox commented Dec 9, 2020

About subdir... I faced this issue before and didnt find a solution for it

See Feature request: Store multiple registered packages in a single Git repository

I know everything in there works from experience. Except I never tried doing the general registration. In the issue the following syntax is suggested: JuliaRegistrator register subdir=path/to/my/package . We have to check if that syntax is actually implemented in JuliaRegistrator, or there is some other syntax now.

Nevermind, JuliaRegistrator mentions it in the Readme, so it should work: https://github.com/JuliaRegistries/Registrator.jl#registering-a-package-in-a-subdirectory

@Andrey1994
Copy link
Member Author

Great! Looks like it was changed since I checked it last time

@matthijscox matthijscox mentioned this issue Dec 15, 2020
7 tasks
@Andrey1994
Copy link
Member Author

@matthijscox I've changed it to tar

@matthijscox
Copy link
Member

Ok, artifact creating can be done much easier I found out with ArtifactUtils.jl

using ArtifactUtils, Pkg.Artifacts
add_artifact!(
           "Artifacts.toml",
           "brainflow",
           "https://github.com/brainflow-dev/brainflow/releases/download/3.7.2/compiled_libs.tar",
           force=true,
       )

Everything should work, except I seem to have 7z.exe unpacking permission errors. I started a Julia discourse discussion here.

@Andrey1994
Copy link
Member Author

idk what can be the reason for unpacking issue...
Where does 7z.exe come from? Is it downloaded automatically by ArtifactUtils? If yes how does it work on linux and mac? Maybe previous approach is better in this case?

@matthijscox
Copy link
Member

matthijscox commented Dec 17, 2020

It's installed together with Julia it seems. Here's the path it uses for me:
C:\Users\matcox\AppData\Local\Programs\Julia-1.5.1\bin\..\libexec\7z.exe

I'll see if I get some response. As workaround I could indeed write a custom __init__() function (or use a build.jl script) for brainflow that installs the files from the url.

Could you try to run that add_artifact! function on your system?

@Andrey1994
Copy link
Member Author

Yeah, I will try on Windows and Linux.
But we definitely should try to find smth that works for all users

@Andrey1994
Copy link
Member Author

ERROR: Could not unpack C:\Users\a1994\AppData\Local\Temp\jl_Suxcpw72Rk into C:\Users\a1994\.julia\artifacts\jl_wW3Ng1
Stacktrace:
 [1] error(::String) at .\error.jl:33
 [2] unpack(::String, ::String; verbose::Bool) at D:\buildbot\worker\package_win64\build\usr\share\julia\stdlib\v1.4\Pkg\src\PlatformEngines.jl:1018
 [3] unpack at D:\buildbot\worker\package_win64\build\usr\share\julia\stdlib\v1.4\Pkg\src\PlatformEngines.jl:990 [inlined]
 [4] #4 at C:\Users\a1994\.julia\packages\ArtifactUtils\LuOIH\src\ArtifactUtils.jl:57 [inlined]
 [5] create_artifact(::ArtifactUtils.var"#4#5"{String}) at D:\buildbot\worker\package_win64\build\usr\share\julia\stdlib\v1.4\Pkg\src\Artifacts.jl:214
 [6] add_artifact!(::String, ::String, ::String; clear::Bool, options::Base.Iterators.Pairs{Symbol,Bool,Tuple{Symbol},NamedTuple{(:force,),Tuple{Bool}}}) at C:\Users\a1994\.julia\packages\ArtifactUtils\LuOIH\src\ArtifactUtils.jl:56
 [7] top-level scope at REPL[4]:1
caused by [exception 1]
failed process: Process(`'D:\Julia\Julia-1.4.0\bin\..\libexec\7z.exe' x 'C:\Users\a1994\AppData\Local\Temp\jl_Suxcpw72Rk' -y -so`, ProcessExited(2)) [2]

Stacktrace:
 [1] pipeline_error(::Base.ProcessChain) at .\process.jl:538
 [2] run(::Base.CmdRedirect; wait::Bool) at .\process.jl:440
 [3] run(::Base.CmdRedirect) at .\process.jl:438
 [4] unpack(::String, ::String; verbose::Bool) at D:\buildbot\worker\package_win64\build\usr\share\julia\stdlib\v1.4\Pkg\src\PlatformEngines.jl:1013
 [5] unpack at D:\buildbot\worker\package_win64\build\usr\share\julia\stdlib\v1.4\Pkg\src\PlatformEngines.jl:990 [inlined]
 [6] #4 at C:\Users\a1994\.julia\packages\ArtifactUtils\LuOIH\src\ArtifactUtils.jl:57 [inlined]
 [7] create_artifact(::ArtifactUtils.var"#4#5"{String}) at D:\buildbot\worker\package_win64\build\usr\share\julia\stdlib\v1.4\Pkg\src\Artifacts.jl:214
 [8] add_artifact!(::String, ::String, ::String; clear::Bool, options::Base.Iterators.Pairs{Symbol,Bool,Tuple{Symbol},NamedTuple{(:force,),Tuple{Bool}}}) at C:\Users\a1994\.julia\packages\ArtifactUtils\LuOIH\src\ArtifactUtils.jl:56
 [9] top-level scope at REPL[4]:1

It was with admin priviligies on Windows

@matthijscox
Copy link
Member

I began a PR: #172
Let's see if we can get it working.

@matthijscox
Copy link
Member

The PR is merged. Now we need to register.

I requested the JuliaRegistrator app be installed for brainflow-dev: https://github.com/apps/juliateam-registrator/installations/new. It may be @Andrey1994 needs to approve, or request yourself.

We can then comment on a commit like here: af142cf#commitcomment-45338833. But that only works once the App is installed.

@Andrey1994
Copy link
Member Author

Andrey1994 commented Dec 22, 2020

Nice!

Is there a way to trigger the registrator manually from command line or somehow else? Ideally I want to register packages from existing CI pipelines which run all tests instead adding julia specific commits. Also in CI pipelines we can change versions if needed otherwise will need to do it manually

@Andrey1994
Copy link
Member Author

As a hack which I really want to avoid we can create commit inside CI pipeline to trigger the registrator app... its not good but better than doing manually

@Andrey1994
Copy link
Member Author

It seems like registrator app just prepares config file and runs smth like

@JuliaRegistrator register branch=name-of-your-branch

I think it can be automated via simple script wo github app

@JuliaRegistrator
Copy link

Error while trying to register: "Branch name-of-your-branch not found"

@Andrey1994
Copy link
Member Author

Seems like there is a simple UI to register packages manually. Have you tried it? It looks like a good option(I like it more than github app)

image

@matthijscox
Copy link
Member

matthijscox commented Dec 22, 2020 via email

@matthijscox
Copy link
Member

@JuliaRegistrator register branch=master subdir=julia-package/brainflow

@JuliaRegistrator
Copy link

Registration pull request created: JuliaRegistries/General/26771

After the above pull request is merged, it is recommended that a tag is created on this repository for the registered package version.

This will be done automatically if the Julia TagBot GitHub Action is installed, or can be done manually through the github interface, or via:

git tag -a brainflow-v3.7.2 -m "<description of version>" 7df8ce511c53830f152bfa953daf2b1d2ba6cf41
git push origin brainflow-v3.7.2

Also, note the warning: This looks like a new registration that registers version 3.7.2.
Ideally, you should register an initial release with 0.0.1, 0.1.0 or 1.0.0 version numbers
This can be safely ignored. However, if you want to fix this you can do so. Call register() again after making the fix. This will update the Pull request.

@matthijscox
Copy link
Member

I do like this github app. What's not to like?

@matthijscox
Copy link
Member

matthijscox commented Dec 22, 2020

Ok I need to update the compat section in the Project.toml

Also the automatic process would like us to follow the Julia style guide, so we should name the package BrainFlow or request a human merge.

Name does not meet all of the following: starts with an uppercase letter, ASCII alphanumerics only, not all letters are uppercase.

@Andrey1994
Copy link
Member Author

Lets rename it for julia to BrainFlow.

In github app I do not like that its not attached to git tags\releases and bypasses CI in fact. And commands to trigger it written somewhere in issue\pr\commit.

Lets keep registrator app if it works

@matthijscox
Copy link
Member

@JuliaRegistrator register branch=master subdir=julia-package/brainflow

@JuliaRegistrator
Copy link

Registration pull request updated: JuliaRegistries/General/26771

After the above pull request is merged, it is recommended that a tag is created on this repository for the registered package version.

This will be done automatically if the Julia TagBot GitHub Action is installed, or can be done manually through the github interface, or via:

git tag -a brainflow-v3.7.2 -m "<description of version>" ba73a0b0834a7e4628e3d796b20b7fa63a58a30e
git push origin brainflow-v3.7.2

Also, note the warning: This looks like a new registration that registers version 3.7.2.
Ideally, you should register an initial release with 0.0.1, 0.1.0 or 1.0.0 version numbers
This can be safely ignored. However, if you want to fix this you can do so. Call register() again after making the fix. This will update the Pull request.

@matthijscox
Copy link
Member

Ok I still made a mistake. compat versions need to have an upper bound.

PackageA = "1" # [1.0.0, 2.0.0), has upper bound (good)
PackageC = ">=3" # [3.0.0, ∞), no upper bound (bad)

I used the latter.

@matthijscox
Copy link
Member

@JuliaRegistrator register branch=master subdir=julia-package/brainflow

@JuliaRegistrator
Copy link

Registration pull request updated: JuliaRegistries/General/26771

After the above pull request is merged, it is recommended that a tag is created on this repository for the registered package version.

This will be done automatically if the Julia TagBot GitHub Action is installed, or can be done manually through the github interface, or via:

git tag -a brainflow-v3.7.2 -m "<description of version>" b503e002c82d4b98fa717d3d1906aeed6871b970
git push origin brainflow-v3.7.2

Also, note the warning: This looks like a new registration that registers version 3.7.2.
Ideally, you should register an initial release with 0.0.1, 0.1.0 or 1.0.0 version numbers
This can be safely ignored. However, if you want to fix this you can do so. Call register() again after making the fix. This will update the Pull request.

@Andrey1994
Copy link
Member Author

Its merged, thanks @matthijscox !

dmitry-sukhoruchkin pushed a commit to neuroidss/brainflow that referenced this issue Feb 15, 2022
* fix ganglion bled accel

* add wifi ganglion accel fixes

* final fix from andrey
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Minor improvements, refactoring, etc enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants