SnoopCompile 1.6.0 is broken

[aminya]

We registered the inner packages, but we did not find time to check if the master will work when the inner packages are registered. We had a hack script that solved this issue, but by removing it now we have a broken master that is tagged as 1.6.

I think we should have registered the inner packages first, and then registered the top-level package separately.

Also, I am getting the error in #104, which we were not getting it before the split.

[KristofferC]

Definitely don't do something like https://github.com/timholy/SnoopCompile.jl/pull/98/commits/bb56316297de70d401e84433bf84064cd9cee9fe

Also, how is it broken? The tests seem to mostly work fine.

[timholy]

Closed by #107

[aminya]

Now that 1.6.1 is registered, I still get errors when running the script online: https://github.com/JuliaMusic/MusicXML.jl/runs/805777098

ERROR: LoadError: ArgumentError: Package SnoopCompileBot not found in current path:
- Run `import Pkg; Pkg.add("SnoopCompileBot")` to install the SnoopCompileBot package.

Apparently, Pkg.add("SnoopCompile") does not add the subdirs. This is a breaking change for the bot API if we have to wait for Pkg to fix this in some later Julia version.

Same thing: https://github.com/aminya/AcuteML.jl/runs/806039699?check_suite_focus=true

[timholy]

It's not different from other packages: if you start with a fresh package repository, and PkgA depends on PkgB, add PkgA also (silently) installs PkgB. At that point, you can say using PkgA or using PkgA.PkgB (assuming PkgA has a using PkgB somewhere in its top-module source) but not using PkgB. Think of add as declaring "which names do you want to have be available in your environment"? If you now say add PkgB, Pkg doesn't have to download any new code, it just makes the hidden version visible.

You can tell that the source code was installed because the stacktrace shows that the errror was thrown from SnoopCompileBot/K9naN/src/snoop_bot.jl:112.

So, looking at snoop_bot.jl, it looks like you migrated everything to use the sub-packages. To maintain backwards compatibility, you would need to have everything work where you never refer to SnoopCompileCore or SnoopCompileBot, everything should be done in terms of SnoopCompile. When starting that split, your best workflow would have been to get everything working using references only to SnoopCompile despite the new internal organization. Once that was achieved, you could consider more fine-grained optimization.

If you want people to be able to do snooping with SnoopCompileCore, then you'll have to change the SnoopCompile.yml to add those packages too. But best would be to make sure that the old way continues to work and then offer this as a new opportunity for optimization.

[KristofferC]

Apparently, Pkg.add("SnoopCompile") does not add the subdirs.

This doesn't make any sense. It adds the package SnoopCompile. To try to explain this more, there is nothing more special about the relation SnoopCompile <-> SnoopCompileBot than e.g. SnoopCompileBot <-> YAML. And just because you add SnoopCompileBot, it doesn't mean that YAML is somehow available to use. If you want to use YAML you add it, if you want to use SnoopCompileBot you add it.

This is a breaking change for the bot API if we have to wait for Pkg to fix this in some later Julia version.

Just to set expectations, there is nothing to fix in Pkg here (except perhaps documentation to clear up confusion like this)

Also, I want to reiterate that https://github.com/aminya/SnoopCompile.jl/blob/depsbuild/deps/build.jl is completely wrong. The build script runs in a separate temporary environment and will not have an effect on the current one. Calling Pkg functions in the build script is fundamentally not what you want to do.

[timholy]

To clarify how you should be testing this: if you want to test a released version of SnoopCompile, you should ]add SnoopCompile and rm, if necessary, any of the subdirectory packages from your environment. Then test the bot.

If you want to test the dev version of SnoopCompile (you'll need to do that to fix the errors), then you should do something similar. Here's a demo starting from blank environment, but you could use your default one if you prefer:

tim@diva:/tmp/sc$ julia -q
(@v1.6) pkg> activate .
 Activating new environment at `/tmp/sc/Project.toml`

(sc) pkg> dev SnoopCompile SnoopCompileCore SnoopCompileAnalysis SnoopCompileBot

Path `/home/tim/.julia/dev/SnoopCompile` exists and looks like the correct package. Using existing path.
Path `/home/tim/.julia/dev/SnoopCompile` exists and looks like the correct repo. Using existing path.
Path `/home/tim/.julia/dev/SnoopCompile` exists and looks like the correct repo. Using existing path.
Path `/home/tim/.julia/dev/SnoopCompile` exists and looks like the correct repo. Using existing path.
  Resolving package versions...
Updating `/tmp/sc/Project.toml`
  [aa65fe97] + SnoopCompile v1.7.0 `~/.julia/dev/SnoopCompile`
  [9ea4277c] + SnoopCompileAnalysis v1.7.0 `~/.julia/dev/SnoopCompile/SnoopCompileAnalysis`
  [1d5e0e55] + SnoopCompileBot v1.7.0 `~/.julia/dev/SnoopCompile/SnoopCompileBot`
  [e2b509da] + SnoopCompileCore v1.7.0 `~/.julia/dev/SnoopCompile/SnoopCompileCore`
Updating `/tmp/sc/Manifest.toml`
  [1520ce14] + AbstractTrees v0.3.3
  [da1fd8a2] + CodeTracking v1.0.0
  [f68482b8] + Cthulhu v1.2.0
  [48062228] + FilePathsBase v0.9.3
  [1eca21be] + FoldingTrees v1.0.0
  [bac558e1] + OrderedCollections v1.2.0
  [aa65fe97] + SnoopCompile v1.7.0 `~/.julia/dev/SnoopCompile`
  [9ea4277c] + SnoopCompileAnalysis v1.7.0 `~/.julia/dev/SnoopCompile/SnoopCompileAnalysis`
  [1d5e0e55] + SnoopCompileBot v1.7.0 `~/.julia/dev/SnoopCompile/SnoopCompileBot`
  [e2b509da] + SnoopCompileCore v1.7.0 `~/.julia/dev/SnoopCompile/SnoopCompileCore`
  [ddb6d928] + YAML v0.4.0
  [2a0f44e3] + Base64
  [ade2ca70] + Dates
  [8ba89e20] + Distributed
  [b77e0a4c] + InteractiveUtils
  [76f85450] + LibGit2
  [8f399da3] + Libdl
  [56ddb016] + Logging
  [d6f4376e] + Markdown
  [44cfe95a] + Pkg
  [de0858da] + Printf
  [3fa0cd96] + REPL
  [9a3f8284] + Random
  [ea8e919c] + SHA
  [9e88b42a] + Serialization
  [6462fe0b] + Sockets
  [8dfed614] + Test
  [cf7118a7] + UUIDs
  [4ec0a83e] + Unicode

(sc) pkg> rm SnoopCompileAnalysis SnoopCompileCore SnoopCompile
Updating `/tmp/sc/Project.toml`
  [aa65fe97] - SnoopCompile v1.7.0 `~/.julia/dev/SnoopCompile`
  [9ea4277c] - SnoopCompileAnalysis v1.7.0 `~/.julia/dev/SnoopCompile/SnoopCompileAnalysis`
  [e2b509da] - SnoopCompileCore v1.7.0 `~/.julia/dev/SnoopCompile/SnoopCompileCore`
Updating `/tmp/sc/Manifest.toml`
  [aa65fe97] - SnoopCompile v1.7.0 `~/.julia/dev/SnoopCompile`

Now you're using the dev version of all the packages but only SnoopCompile is known in the environment:

tim@diva:/tmp/sc$ cat Project.toml 
[deps]
SnoopCompileBot = "1d5e0e55-7d74-4714-b8d8-efa80e938cf7"
tim@diva:/tmp/sc$ cat Manifest.toml 
# This file is machine-generated - editing it directly is not advised

⋮

[[SnoopCompileAnalysis]]
deps = ["Cthulhu", "OrderedCollections", "Serialization"]
path = "/home/tim/.julia/dev/SnoopCompile/SnoopCompileAnalysis"
uuid = "9ea4277c-da97-4c3a-afb0-537c066769de"
version = "1.7.0"

[[SnoopCompileBot]]
deps = ["FilePathsBase", "Pkg", "SnoopCompileAnalysis", "SnoopCompileCore", "YAML"]
path = "/home/tim/.julia/dev/SnoopCompile/SnoopCompileBot"
uuid = "1d5e0e55-7d74-4714-b8d8-efa80e938cf7"
version = "1.7.0"

[[SnoopCompileCore]]
deps = ["Serialization"]
path = "/home/tim/.julia/dev/SnoopCompile/SnoopCompileCore"
uuid = "e2b509da-e806-4183-be48-004708413034"
version = "1.7.0"

⋮

among many other packages.

As long as you keep the mantra, "(currently) there is nothing special about having all sub-packages be part of the same repo," then these rules might be more intuitive since this is how it works generally, not just in this particular case.

[aminya]

Thanks for the answers. I want all the sub-packages to be available under the global environment. For backward compatibility, I need to add a script to add those sub-packages inside SnoopCompile.

The only reason that the top-level SnoopCompile now exists, is for backward compatibility. So, we assume that people will only add that to their environment. Changing the workflow file can break the API. When we made sure that the old workflow works, we can switch to the new workflow that adds all of the sub-packages.

@timholy I prefer to fix the compatibility by making the sub-packages available when someone adds SnoopCompile, instead of reverting my code to be old just for that matter. If there is a way to fix this without reverting my code to pre-split era, I would love to use that instead.

[aminya]

Also, I want to reiterate that aminya/SnoopCompile.jl:deps/build.jl@depsbuild is completely wrong. The build script runs in a separate temporary environment and will not have an effect on the current one. Calling Pkg functions in the build script is fundamentally not what you want to do.

My scripts run in an external process. I need all the packages to be available in the global environment (See install dependencies step). For compatibility, I have to make this happen when people do Pkg.add("SnoopCompile").

          julia -e 'using Pkg; Pkg.add(PackageSpec(url = "timholy/SnoopCompile.jl")); Pkg.develop(PackageSpec(; path=pwd())); using SnoopCompile; SnoopCompile.addtestdep();'

using Pkg
Pkg.activate()  # Even when I run `Pkg.activate()` before adding these, they are not available in the global environment.

Pkg.add([
  "SnoopCompileCore",
  "SnoopCompileAnalysis",
  "SnoopCompileBot",
])
Pkg.resolve()

Although the packages are added, they are not still available in the global environment like a normal package would, and running using SnoopCompileBot errors:

AcuteML: https://github.com/aminya/AcuteML.jl/pull/141/checks?check_run_id=812295621 MusicXML: https://github.com/JuliaMusic/MusicXML.jl/actions/runs/148956710

Details

```julia Run julia --project -e 'include("deps/SnoopCompile/snoop_bot.jl")' [ Info: /home/runner/work/AcuteML.jl/AcuteML.jl/src/precompile_includer.jl file will be created/overwritten [ Info: SnoopCompile will try to write "include("precompile_includer.jl")" before end of the module in /home/runner/work/AcuteML.jl/AcuteML.jl/src/AcuteML.jl. Assume that the last `end` is the end of a module. ERROR: LoadError: ArgumentError: Package SnoopCompileBot not found in current path: - Run `import Pkg; Pkg.add("SnoopCompileBot")` to install the SnoopCompileBot package. Stacktrace: [1] require(::Module, ::Symbol) at ./loading.jl:876 [2] top-level scope at /home/runner/.julia/packages/SnoopCompileBot/K9naN/src/snoop_bot.jl:112 [3] eval at ./boot.jl:330 [inlined] [4] #snoop_bot#8(::Symbol, ::typeof(snoop_bot), ::BotConfig, ::String, ::Module) at /home/runner/.julia/packages/SnoopCompileBot/K9naN/src/snoop_bot.jl:182 [5] snoop_bot at /home/runner/.julia/packages/SnoopCompileBot/K9naN/src/snoop_bot.jl:177 [inlined] (repeats 2 times) [6] top-level scope at /home/runner/work/AcuteML.jl/AcuteML.jl/deps/SnoopCompile/snoop_bot.jl:8 [7] include at ./boot.jl:328 [inlined] [8] include_relative(::Module, ::String) at ./loading.jl:1094 [9] include(::Module, ::String) at ./Base.jl:31 [10] include(::String) at ./client.jl:431 [11] top-level scope at none:1 in expression starting at /home/runner/work/AcuteML.jl/AcuteML.jl/deps/SnoopCompile/snoop_bot.jl:8 ```

Maybe I have to change this script here: https://github.com/timholy/SnoopCompile.jl/blob/0aa0daea5c54a9ad4050ccf3a29bf60a301c7f24/SnoopCompileBot/src/snoop_bot.jl#L112

This is called from Generating precompile files step:

        run: julia --project -e 'include("deps/SnoopCompile/snoop_bot.jl")'   # NOTE: must match path

I am not sure why this environment does not have SnoopCompileBot in it!

[timholy]

@aminya, my point is that we called this release 1.6.0. The intent was that old scripts should continue to work. If they don't work, we have to do some compatibility work. It's not fair to ask everyone who has been using SnoopCompile to change their scripts for a 1.5->1.6 transition. It's quite important that we do this. Otherwise we really needed to call that release 2.0, but we didn't. I'm not sure that we can remove the 1.6 release from the registry, for example.

It's of course fine to also move towards a new way of doing things, but the whole meaning of backwards-compatible is that the old ways have to continue working without modification.

Can we get a fix out soon?

[aminya]

I don't know what to do at this point. I am literally adding all the packagees before using them everywhere, and it still errors

        using Pkg
        Pkg.add([
          "SnoopCompileCore",
          "SnoopCompileAnalysis",
          "SnoopCompileBot",
        ])
        Pkg.resolve()

        using SnoopCompileBot

https://github.com/aminya/AcuteML.jl/runs/812801777#step:5:7

[timholy]

No, don't do that! I showed you how to set up your local machine in https://github.com/timholy/SnoopCompile.jl/issues/105#issuecomment-649345931, where:

• you're using devved versions of SnoopCompile and all its submodules, BUT
• your environment only knows about SnoopCompile itself

This is the key thing to replicate the environment that your "clients" will have, while also allowing you to try code locally that has not been registered. (Your clients will not have that problem because they will get the new code after it has been registered.) Do this, then do not touch your environment anymore.

Then, fix SnoopCompile so that the scripts work.

[aminya]

This error is happening online, and so their environment is clean! I am not using a dev version here. I am testing the end-result online.

With my final brute-force solution failing, I have no clue what to do.

What does this mean? Does it mean that people cannot run using SnoopCompileBot in some environment? Is there a way to add a package to the global environment, and using it in an external process? These submodules might be different from normal packages, which makes all of these problems.

[timholy]

Ignore errors that happen online. SnoopCompile is broken, and until we release a fix packages are doomed to use the broken version. The only thing that matters is whether you can run the old scripts on your local machine, because on your local machine you can use the trick in https://github.com/timholy/SnoopCompile.jl/issues/105#issuecomment-649345931 to be running a new-and-improved SnoopCompile (one that only you have access to). Once you get it working locally, submit a PR here, we'll register a new release, and then everything should work online again.

As a way of fixing the problem, note that

using SnoopCompile
@snoopi stuff

still works. So do that for now. Don't use the submodules directly until you can get it working with the meta-package.