Random 'Invalid query string' after first login with example

I executed the stateless application install, and created a new user, and when I log in, most of the time I'm presented with the following error:
ERROR: Error: Invalid query string. Your query string should include parameters "state" and "code"
the state parameter is missing from the call. If I press the provided "refresh" link, sometimes it works and I'm passed into the applications success screen. Is there any way to fix this?
After logging in, I always land on https://CFID.cloudfront.net/parseauth?code=f3c82b7a-3c4f-4386-9cb7-19b894cd3698 with Bad Request - ERROR: Error: Invalid query string. Your query string should include parameters "state" and "code"

That shouldn't happen!
The URL you provided above isn't right, it doesn't contain a state, only a code.
How did you deploy the application? Through the serverless application repository or manually using CLI? (Question behind the question is, are you on the latest version of the code?)
If you are indeed on the latest version, can you open the network panel of your browser, and explain what you see going on in there? Which redirects are triggered, using which path patterns, etc.

I used:

Option 1: Deploy through the Serverless Application Repository
I made no change to the 'Application settings', beside checking the box giving permissions.

After I goto Cognito and press the Open UI button (and then log in):
My network log has two hits...First:
https://auth-XXXXXXXXXX.auth.us-east-1.amazoncognito.com/login?client_id=2ngojgeb5m8kehi009llqlmk25&response_type=code&scope=aws.cognito.signin.user.admin+email+openid+phone+profile&redirect_uri=https://ZZZZZZZZZZ.cloudfront.net/parseauth
Responds with a 302
Then https://ZZZZZZZZZZ.cloudfront.net/parseauth?code=de477e41-1bbc-4a07-b1ed-3a9746c86cd8 is hit, missing the state parameter (at least from my understanding), and returns a 400 stating:

Bad Request
ERROR: Error: Invalid query string. Your query string should include parameters "state" and "code"

Try again

So the Cognito UI will call /login first asking for authentication, and then it replies with the code, and redirects it to the /parseAuth call. But because it hasn't hit check auth yet, there is no state yet. Does parseauth need better checking for this case, and then do another redirect around checkout (either the URL or the similar code) ?

Ahh yes that's the issue. Opening the app using that link in Cognito User Pool doesn't work. You need to just go to the URL of the web app (as users would).
However the "Try again" button does bail you out right? To be honest, that seems fine then to me, as only admins would use that button in the Cognito UI.
On the other hand, other admins will run into this to, and might have the same question as you had. We could add that explanation to the error message "you probably got here through the button in Cognito ..... press Try again". Agree that that's good enough?
There might be other reasons why state wouldn't be present in the query string, think it is better to not do the "try again" automatically, and keep visibility of this situation.

My use case is that, each one of those "Open Cognito UI" buttons allows the passing of a client_id, which tells Cognito specifically "use this IDP" or "Use this User Pool". So ideally, on their IDP, I'll send them to the domain + query parameter. I think the solution is to add the ability to forward this parameter from the initial domain + query parameter into checkauth and have that call /authenticate with the code, which would then authenticate them though the correct portal.

Also in terms of the larger project, wouldn't it be better to modify parseauth so that it can handle this case, such that it yields a successful login for the user? That would be a better UX.

The state contains a nonce, which protects against CSRF. Other sites cannot just redirect users to the protected site, that is by security design.
It sounds though like that is precisely what you want to do, which is fine - if it makes sense in your use case.
Can you describe your use case in more detail? You want to federate Cognito to multiple IDP's, and have links that would go directly to any one of those IDP's instead of the Cognito Hosted being opened showing all the options? Is that it? Why is it important (I can guess but interested to hear you on this)

Well each client would have their own client_id, and thus their own separate logins, with the Cognito User Pool login disabled. So having a client_id is required to be presented with the right IDP Cognito authentication page. Going to the URL directly would yield a blank page telling the user to go away (or something nicer). If I have 20 clients, I don't want 20 buttons that say "Login with X", "Login with Y", etc. I don't want my users to have to expose their IDP login systems to everyone else using the system.

OKay makes sense.
And why one User Pool and one CloudFront distribution? Any particular reason?

To me it made the most logical sense because if I had 200 different clients for my app, I'd have to have 200 distributions, plus 200 x 5 lambda functions for each client. (Since the lambdas are not re-usable in this fashion). So by having one Distribution, and one SPA, but multiple clients from the Cognito IDP stand-point, users can only authenticate with their IDP and I can use the identity providers Identifiers to know which client is logging into which backend.

That would also mean 200 IDP's if I understand you correctly. Is that in any way realistic for you, can you shed some light on that? I'd like to talk real business use cases, and fully understand those.
The clientId is encoded in the Lambda functions now. So using multiple client ID's would require changing the way the Lambda's work.
A client would normally map to a SPA, you would not use multiple Client ID's for the same SPA. I mean, technically you can, but it it sounds like you're only doing it to have separate login URL's per IDP. That I can understand, but can also be solved with one Client ID. We would need to come up instead then with a way to let the checkAuth handler know what IDP it should redirect to.
At this point though, it doesn't sound like this requirement is generic enough, to be implemented here in this repo. But I'm happy to be convinced otherwise.

I see, I guess I didn't realize till now that the lambdas have the client id hard coded into them. The business case for the multiple IDPs is that many of our clients have their own SAML and we want to partner with them to allow them to connect their SAML to our Cognito provider, and thus give them access to our app via their authentication. For now we have a few clients, but in the future it could go way up.
At this point the only option with this is to have multiple clients, but then use multiple CloudFront instances, with multiple lambdas.

A way to do it:

have one client, one user pool, one cloudfront dist
create the federation IDP's, give each one a unique ID, readable or obfuscated (what you want)
update the checkauth lambda so that it reads the URL query param "idp_identifier". If it is not there throw error, if it is there (and user is not signed in) redirect to cognito and pass along the idp_identifier param. If that is present Cognito will not show HostedUI  but go to that IDP directly.

So e.g. if you had setup federation for company A, name that IDP companyA and then with above change in checkAuth in place the user has to go to e.g.: http://example.com/some/path?idp_identifier=companyA
Read here about that param "idp_identifier": https://docs.aws.amazon.com/cognito/latest/developerguide/authorization-endpoint.html
Easy enough I think, just have to change the checkAuth handler.
Probably if we keep thinking we'll find some other options too.

With this method though, there would still be the Cognito login page, with a bunch of buttons displaying all the different providers, right ?

I don't want to turn this into a support issue, but do you happen to see what I'm doing wrong here. I added the idp_identifier code, and passing it to /authorize.... and I'm getting back: ERROR: Error: [Cognito] invalid_request: invalid_scope
This is an example of the URL I'm sending, can you see anything wrong: (I decoded it to clean it up for you, but its encoded when sent to the server):
redirect_uri=https://d2tbgx16w63u2h.cloudfront.net/parseauth&response_type=code&client_id=123d4ml5pj67n8f9rmupcllelr&state=eyRub25jZSI6IlF1QXg4TmduQVJ5ZGAMEjciLCOyZXF1ZCK0ZWRVcmkiOiIvP2lkcF9pZGVudGlmaWVyPWNsZWFyLW9rdGEtaWRlbnRpZmllciJ9&scope=phone email profile openid aws.cognito.signin.user.admin&code_challenge_method=S256&code_challenge=8Y4W_eYh7cH-ihCA7ZV4uXb8ApOe9tArTfAM1xMOUI4&idp_identifier=clear-okta-identifier

The URI looks good to me. Did you also allow all those scope on the User Pool client?
 So it was something like this before? "&scope=phone%20email%20profile%20openid%20aws.cognito.signin.user.admin&..."
That should be good.

I thought I did that, but I re-applied them and it works!

Great to hear!
Will close this issue for now, don't hesitate to report back if you run into anything else.

Wait, still have to improve the error message.

Added nicer err msg in #48


The recent typings change made me difficult to define Reducers in Typescript.

I open this issue because I don't think this is a normal Usage Question, I want to talk why do we change it.
I found we have a typings change recently: (https://github.com/reduxjs/redux/pull/2291 TypeScript typings: reducer state can be undefined)
According to the above issue, I think the point is that we don't need to check the undefined with state, but why do we change the typings at the end?
In the past, I would write a Reducer like:
export const loginRequest: Reducer<ImmutableAuthenticationState> =
  (state: ImmutableAuthenticationState, {payload}: AnyAction & { payload?: LoginRequestParams }) =>
    payload ? state.merge({ /* something */ }) : state);

But for now, I have to make it very redundant and complex:
export const loginRequest: Reducer<ImmutableAuthenticationState> =
  (state: ImmutableAuthenticationState | undefined, {payload}: AnyAction & { payload?: LoginRequestParams }) =>
    state ? (payload ? state.merge({ /* something */ }) : state) : INITIAL_STATE;

What's wrong?

You need to be providing a default value to the state. That's why the undefined was introduced.


TamilPesu Web API for sandhi checker processes same string

TamilPesu Web API for sandhi checker processes same string, since it uses default POST request whereas HTML page directs a GET request to it.

This issue is different from #165 and resolved by commit bde472d8488079e34fdb257b692f17e97c02fae7


Are you still interested in maintaining this?

I've recently discovered the utility of indexed monads, and it's a shame that this package doesn't use polykinds (#1).
As it's been some years since the last release, and approaching a month since the current issues were opened, I was wondering if you were still interested in maintaing this package? If not, I'd be happy to take it off your hands.

Bump; i also need polykinds.

+1 for polykinds.

My apologies for the delay. I am now reviewing all issues filed here.
Polykinds support makes sense, with one possible concern: we lose Haskell98 support. Indeed, that was the original motivation behind splitting into 'indexed' and 'indexed-extras'.
From reading the Polykinds documentation it seems I need to require Polykinds support at the declaration site (of the type or typeclass), which affects all callers, even ones who don't need it. Is there a technical way around this?
If there's no technical solution, then I suspect we may as well give up Haskell98 support, but it would be helpful if someone with more experience about polykinds could chime in.

I don't think there is a way around requiring PolyKinds at the declaration site. Although it doesn't create an incompatibility with Haskell98, as a Haskell98 implementation will just see {-# LANGUAGE PolyKinds #-} as a comment and ignore it.

Thanks for the suggestion. I've done it in a backwards-compatible way in ca1cb0f268fe1cd03df5e047fac124f55cf95066.


Allow users to convert published Answer editions to Simple Smart Answer editions

The body of the previous edition is copied across.
https://www.pivotaltracker.com/n/projects/1261204/stories/89923420

:+1:


ARN-2066 - Fix routing and validation logic for employment and education

Fixes some weirdness with validation/routing for questions with shared codes.

Maintain values keyed by ID for use in validation, this is used as the key for validation errors
Update getAnswersToAdd to first attempt to source an answer by ID (local), falling back to the answer by code (persisted)

Does this affect the summary pages? Lines 29, 120, 121 in nunjucks.summaryFields.ts

It shouldn't I've had a look through and this section of code is referring to the code which should be the keys answers are persisted against in the backend


mobile: Remove golang.org/x/mobile dependencies if possible

For JNI access, JNI_OnLoad might be available.
For example: https://git.sr.ht/~eliasnaur/gio/tree/master/app/os_android.c

The challenging is that this requires to remove all the dependencies (including Oto) on golang.org/x/mobile/app, or JNI_OnLoad definition would be duplicated.


[SILGen] Utilize _silgen_name for implicit global variables

The known __dso_handle symbol name wasn't getting used when emitting
references to #dsohandle, causing link time failures for a mangled
name that didn't exist.
rdar://problem/26565092

@swift-ci Please test

Git clone failure - trying again in a few minutes.

@swift-ci Please test

Ah, it looks like swift-3.0-preview-4-branch doesn't exist yet in https://github.com/apple/swift-compiler-rt. @zisko, do you know who is doing that by any chance?

It may be that the clone is hosed on https://ci.swift.org/computer/Mac-OSX-28/.

@swift-ci Please test


Type aliases not working as expected

Description
Type aliases do not work (intuitively).
Impact
Yes, this is blocking .
Input specification
---- MODULE main ----

EXTENDS Integers, FiniteSets, Sequences, TLC, Apalache

(*

    @typeAlias: X = Str;

*)

CONSTANTS
    \* @type: Set(X);
    XS

VARIABLES
    \* @type: Int;
    a

CInit == XS = {"k", "u", "v"}

Init == a = 42

Go == a' = 42

Next == Go

Inv == TRUE

====

The command line parameters used to run the tool
apalache typecheck main.tla
Expected behavior
It to typecheck.
Log files
15:32:22.608 [main] INFO at.forsyte.apalache.tla.tooling.opt.TypeCheckCmd - Loading configuration
# Usage statistics is OFF. We care about your privacy.
# If you want to help our project, consider enabling statistics with config --enable-stats=true.

Output directory: /Users/danwt/Documents/work/interchain-security/x/ccv/provider/keeper/prototyping/model/_apalache-out/main.tla/2022-09-20T15-32-22_13182915876821651716
# APALACHE version: 0.29.1 | build: 146de56                       I@15:32:22.770
Type checking main.tla                                            I@15:32:22.779
PASS #0: SanyParser                                               I@15:32:22.868
Parsing file /Users/danwt/Documents/work/interchain-security/x/ccv/provider/keeper/prototyping/model/main.tla
Parsing file /private/var/folders/9l/fjtclx4d6txfdsj2ynqnrr5c0000gn/T/Integers.tla
Parsing file /private/var/folders/9l/fjtclx4d6txfdsj2ynqnrr5c0000gn/T/FiniteSets.tla
Parsing file /private/var/folders/9l/fjtclx4d6txfdsj2ynqnrr5c0000gn/T/__rewire_sequences_in_apalache.tla
Parsing file /private/var/folders/9l/fjtclx4d6txfdsj2ynqnrr5c0000gn/T/__rewire_tlc_in_apalache.tla
Parsing file /private/var/folders/9l/fjtclx4d6txfdsj2ynqnrr5c0000gn/T/Apalache.tla
Parsing file /private/var/folders/9l/fjtclx4d6txfdsj2ynqnrr5c0000gn/T/Naturals.tla
Parsing file /private/var/folders/9l/fjtclx4d6txfdsj2ynqnrr5c0000gn/T/__apalache_folds.tla
PASS #1: TypeCheckerSnowcat                                       I@15:32:23.070
 > Running Snowcat .::.                                           I@15:32:23.070
[main.tla:19:10-19:29]: Arguments to = should have the same type. For arguments XS, {"k", "u", "v"} with types Set(X), Set(Str), in expression XS = {"k", "u", "v"} E@15:32:23.371
[main.tla:19:1-19:29]: Error when computing the type of CInit     E@15:32:23.373
 > Snowcat asks you to fix the types. Meow.                       I@15:32:23.373
Type checker [FAILED]                                             I@15:32:23.374
It took me 0 days  0 hours  0 min  0 sec                          I@15:32:23.374
Total time: 0.603 sec                                             I@15:32:23.374
EXITCODE: ERROR (120)

System information

Apalache version [apalache-mc version]: 0.29.1 | build: 146de56
OS [e.g. Ubuntu Linux or Mac OS, and the current version]: OSX
JDK version [e.g. OpenJDK 0.8.0]: OpenJDK 64-Bit Server VM Zulu17.30+15-CA (build 17.0.1+12-LTS, mixed mode, sharing)

Additional context

Triage checklist (for maintainers)

[ ] Reproduce the bug on the main development branch.
[ ] Add the issue to the apalache GitHub project.
[ ] If the bug is high impact, ensure someone available is assigned to fix it.

The following changes produce a type correct spec:
---- MODULE main ----

EXTENDS Integers, FiniteSets, Sequences, TLC, Apalache

(*

    @typeAlias: x = Str;
*)
TypeAliases == TRUE

CONSTANTS
    \* @type: Set($x);
    XS

VARIABLES
    \* @type: Int;
    a

CInit == XS = {"k", "u", "v"}

Init == a = 42

Go == a' = 42

Next == Go

Inv == TRUE

====

The needed changes:

the alias declaration needs to associated with an operator (or else the comment isn't preserved for us to extract the annotation).
type aliases now have to be in camel case (so can't start with a caps)
references to type aliases must now start with a $

These finicky details are recorded in the manual https://apalache.informal.systems/docs/adr/002adr-types.html?highlight=type aliase#12-type-aliases
We discussed a it with @danwt in a different context, and all agreed that better error messages would help users with this kind of thing. See #955 and #943.


Suggestion to improve accessibility

Awesome job on the python implementation!
I just have one suggestion. I feel a quick way for anyone to know what this project is about would be to provide an easy way (maybe a few lines of instruction in the README) for users to process their own images.

I'd like to have an example how to process my owns images.
I tried to change your code but it didn't work.
Ex: process.py myImage.jpg and result is a image processed in another file (result_myImage.jpg)


Don't pin versions in setup.py

See #580

See #580

was merged. Closing this.

Nope: https://github.com/bigchaindb/bigchaindb/blob/master/setup.py#L93-L101


ci: doc-build: Reduce parallel job count to 14

This commit reduces the parallel job count for building the documentation from the default value of 16 (i.e. -j auto aka. the vCPU count) to 14 because the total available RAM in the runners is 32GiB and each sphinx-build process may use more than 2GiB of RAM, which can lead to out-of-memory conditions.
Note that the HTML doc build process was parallelised prior to this change in spite of the lack of -j auto in the CI workflow because the default SPHINXOPTS includes -j auto and only SPHINXOPTS_EXTRA was specified -- this commit explicitly adds the SPHINXOPTS for it with the default value specified in doc/CMakeLists.txt to make the job count configurable while ensuring the previous behaviour is preserved.

Not directly related to this PR, but why do we even need to override SPHINXOPTS="-j ${JOB_COUNT} like this, rather than being able to provide the j value when calling make?
This has been a thorn in the side for many people who have many cores but not enough memory trying to build the documentation, where a solution that allows users to do make -j 4 html would be a far better solution.
Is it not possible to do that?


Can we delete release branches?

@tseaver We have
https://github.com/GoogleCloudPlatform/gcloud-python/tree/release-0.16.1
https://github.com/GoogleCloudPlatform/gcloud-python/tree/releases-0.18.x
and I think they've already served their purpose? If we want an actually pinned commit we can use tags, not branches?

@tseaver Bump

The release-0.16.1 branch can certainly go:  the chance that we will make a 0.16.2 release is infinitessimal.  Until we've made a 0.19.0 release, I would keep the release-0.18.x branch around, on the chance that there is some lingering bug (or another upstream screwup) which might require a 0.18..2 release.

@tseaver, couldn't we just branch off the 0.18.1 tag to make a new release
with a bugfix?
On Tue, Aug 23, 2016 at 9:30 AM, Tres Seaver<EMAIL_ADDRESS>wrote:

The release-0.16.1 branch can certainly go: the chance that we will make
a 0.16.2 release is infinitessimal. Until we've made a 0.19.0 release, I
would keep the release-0.18.x branch around, on the chance that there is
some lingering bug (or another upstream screwup) which might require a
0.18..2 release.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/GoogleCloudPlatform/gcloud-python/issues/2146#issuecomment-241730635,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AALvyLxa2WjvQ2N-XShoOP7Q6hBRXeGcks5qivXrgaJpZM4Jov3r
.

--
Tom Schultz

Just deleted release-0.16.1. @tseaver Bump on the other one

I've deleted the branch, after verifying that it had no drift from 0.18.1.


Feature/DirectToBTRL

As we've discussed.

ModTek.zip at RN

Moving to https://github.com/BattletechModders/ModTek/pull/86


Urgent: Adding Multiple Filter Fields in Backgrid ServerSide Filter

I would like to add two checkboxes with Search text boxes as a filter through Backgrid. is there any option is exist or any API is exist or we have append through jQuery after search filter textbox ?
or How to extend the Backgrid.Extension.ServerSideFilter?

https://gist.github.com/martindrapeau/9812237


Adjust precompile workload to avoid warnings

1.10 during precompilation has some warnings due to tasks hanging. This is due to the Malt workers. Skip these parts of the precompilation so that we don't run into that warning.
Fixes #8.

Doesn't appear to affect latency much at all, so this'll be fine.

Do we understand why it hangs? What does Malt itself do for precompilation?

Starts up worker processes that don't immediately close from what I can tell. It's not a critical issue since they do eventually close (and don't hang indefinitely), it just looks weird seeing the warning in precompilation.

What does Malt itself do for precompilation?

Nothing at all.


OKTA as Identity Broker

As an administrator of ZITADEL I want to be able to allow my users to use their OKTA account login, so they do not have to create a new account.
Acceptance criteria

[ ] I am able to add OKTA as an identity provider to my instance and organization
[ ]  I only have to configure client id and client secret, the rest is provided by ZITADEL (issuer, etc)
[ ] I am able to login with my OKTA account
[ ] If apple does have some design requirements, the login button does show it
[ ] OKTA is implemented as provider based on our identity provider templates

Depends on https://github.com/zitadel/zitadel/issues/2998

OKTA is currently possible as generic OIDC Provider: https://zitadel.com/docs/guides/integrate/identity-providers/okta#okta-configuration


Bring maybestd module back

Hiding maybestd makes it extremely inconvenient to write
no_std-compatible libraries with borsh serialisation.  In the past it
was possible to do:
impl borsh::BorshSerialize for Foo {
    fn serialize<W: borsh::maybestd::io::Write>(
        &self, writer: &mut W,
    ) -> borsh::maybestd::io::Result<()> {
        todo!()
    }
}

Now, there’s borsh::nostd_io however if one uses that module their
code breaks as soon as borsh’s std feature is enabled.  This forces
library writer to have their own std feature (something they may no
need for) which enables borsh/std and conditionally uses std::io
or borsh::nostd_io depending on it; and forces library users to now
enable that feature in the library.
While at it, simplify __private module a bit by moving stuff out of
__private::maybestd module up a level.

We don't want to turn borsh-rs into maybestd provider.

I mean, you should have thought about that when you decided to use io::Read, io::Write and io::Error as part of borsh’s public API.  For comparison, serde does offer StdError which switches between std::io::Error and its own trait depending on Cargo features.
Not offering maybestd is not only hostile to users of borsh but also to all users of no_std-compatible libraries which use borsh.

consider using use borsh::__private::maybestd; - it is still public, but from a borsh crate point of view, it is a private implementation that we don't want to provide backward compatibility guaranties.

But you are providing compatibility guarantees to borsh::nostd_io, correct? So you’re already paying 99% of the cost of supporting borsh::maybestd.  Adding borsh::maybestd::io as re-export of borsh::nostd_io or parts of std::io is a rather tame requirement.

But you are providing compatibility guarantees to borsh::nostd_io, correct?

That scope is small and self-contained in a single file, and that is indeed part of the public interface, as you stated. maybestd does not need to be part of the public interface, hence the difference.

Not offering maybestd is not only hostile to users of borsh but also to all users of no_std-compatible libraries which use borsh.

no_std-compatible libraries have two valid options:

Treat themselves as "friend" modules (in C++ terms), and use borsh::__private::maybestd wherever applicable (borsh-derive does that), knowing that they need to pay attention to __private changes
Write a minimal maybestd implementation themselves, as you said, it will be just a re-export

We optimize borsh for application developers, and we don't want them to abuse borsh::maybestd as they will later come and request more re-exports there.

That scope is small and self-contained in a single file,

Just like what this PR proposes:
pub mod maybestd {
    pub mod io {
        #[cfg(feature = "std")]
        pub use std::io::{Error, ErrorKind, Read, Result, Write};

        #[cfg(not(feature = "std"))]
        pub use crate::nostd_io::*;
    }
}

Literally five symbols which are part of borsh’s public API.  If you don’t like the name maybestd I’d also be happy with:
#[cfg(feature = "std")]
pub mod io {
    pub use std::io::{Error, ErrorKind, Read, Result, Write};
}

#[cfg(not(feature = "std"))]
pub use crate::nostd_io as io;

That would offer types borsh::io::Read etc.

Treat themselves as "friend" modules (in C++ terms), and use borsh::__private::maybestd wherever applicable (borsh-derive does that), knowing that they need to pay attention to __private changes

What’s the point of borsh::nostd_io then?  At the moment it’s a foot-gun breaking people’s builds.  Consider the following pascal-str crate:
[package]
name = "pascal-str"
version = "0.1.0"
edition = "2021"

[dependencies]
borsh = { version = "1.0.0-alpha.4", default-features = false }

#![no_std]
extern crate alloc;

struct PascalString(alloc::vec::Vec<u8>);

impl borsh::BorshSerialize for PascalString {
    fn serialize<W: borsh::nostd_io::Write>(&self, writer: &mut W) -> borsh::nostd_io::Result<()> {
        writer.write_all(&[self.0.len() as u8])?;
        writer.write_all(&self.0)
    }
}

It compiles and works fine.  But let’s now say someone uses it in std application like so:
[package]
name = "foo"
version = "0.1.0"
edition = "2021"

[dependencies]
borsh = "1.0.0-alpha.4"
pascal-str = { path = "../pascal-str" }

And suddenly the code breaks:
error[E0433]: failed to resolve: could not find `nostd_io` in `borsh`
 --> /tmp/b/pascal-str/src/lib.rs:7:28
  |
7 |     fn serialize<W: borsh::nostd_io::Write>(&self, writer: &mut W) -> borsh::nostd_io::Result<()> {
  |                            ^^^^^^^^ could not find `nostd_io` in `borsh`

error[E0433]: failed to resolve: could not find `nostd_io` in `borsh`
 --> /tmp/b/pascal-str/src/lib.rs:7:78
  |
7 |     fn serialize<W: borsh::nostd_io::Write>(&self, writer: &mut W) -> borsh::nostd_io::Result<()> {
  |                                                                              ^^^^^^^^ could not find `nostd_io` in `borsh`

Enabling a cargo feature shouldn’t break builds.

We optimize borsh for application developer

Except you don’t.  Because now author of pascal-str has to introduce a std feature to their crate and application developer has to enable it.  This is an objectively worse experience for application developer than what they had with borsh 0.10.

Write a minimal maybestd implementation themselves, as you said, it will be just a re-export

But as shown, it’s not just a matter of a simple maybestd module with re-exports.  I have no way to refer to Write, Read, Error and ErrorKind symbols used by borsh.  I have to have user of my crate tell me how borsh was compiled.

and we don't want them to abuse borsh::maybestd as they will later come and request more re-exports there.

If they do, tell them to GTFO.  However, if something is part of borsh’s public API that should be provided by borsh in a way which doesn’t depend on whether std feature is enabled or not.

Because now author of pascal-str has to introduce a std feature to their crate and application developer has to enable it. This is an objectively worse experience for application developer than what they had with borsh 0.10.

Well, pascal-str might want to have std feature anyway to enable it in borsh, but I see your point now.

But as shown, it’s not just a matter of a simple maybestd module with re-exports. I have no way to refer to Write, Read, Error and ErrorKind symbols used by borsh. I have to have user of my crate tell me how borsh was compiled.

This is a fair point. @mina86 @dj8yfo @matklad I would like to consider having borsh::maybestd_io or borsh::io module to be part of the public API. Still, I don't want to expose the whole maybestd. What do you think?

Well, pascal-str might want to have std feature anyway to enable it in borsh...

Agree.

I have no way to refer to Write, Read, Error and ErrorKind symbols used by borsh.

Using this recipe https://github.com/near/borsh-rs/blob/09c9295000ced1933e067d094a3cd8fe5507044d/borsh/tests/test_ser_de_with.rs#L22-L26 should help.
Doing the dispatch on the client lib side is more explicit about what's happening and i like it more.

like to consider having borsh::maybestd_io or borsh::io module to be part of the public API

This might result in people mindlessly using borsh::io  instead of std::io even in the spots completely unrelated to borsh (IDE completion somehow adversely affects minds of folks).
Here's an example https://github.com/near/nearcore/pull/9432/files#diff-5610719ce9d54675a9153a04627f8988705312f7dec97375fa702283ed796c1aL4 and there're a few more.
This pr has most likely protected from cases when items unrelated to borsh::io might be used by only reexporting the relevant ones https://github.com/mina86/borsh-rs/blob/2e942c8c88611804db699237d6b0d976da40d3c4/borsh/src/lib.rs#L127-L130 .
But it doesn't protect from cases when borsh::io is used in wrong context, where std::io was meant instead.
Supposedly, the recipe above addresses this too.
We've already adjusted the plan of #51 with making schema -> unstable__schema due to additions by @mina86.
Hopefully, he can sustain some backpressure in client libraries too.

Build breaks when std feature is toggled.  This is a bug.
There’s no way to name Read, Write, Result, Error and ErrorKind types used by borsh thus making life of library developers harder.  This is bad API design and user-hostile.
Library developers are forced to introduce std feature which makes life of application developers harder since they now have to enable that feature on libraries which otherwise wouldn’t have it.  This is user-hostile and contradicts frol’s statement that borsh is optimised for application developers.

The fact that pascal-str might want a std feature anyway is not an argument.  First of all, there are many possible crates which won’t have std feature.  Specifically the hypothetical pascal-str crate would probably not have that feature since it would be a wrapper around alloc::string::String.  Second of all, there’s a bug in borsh.  I’ve presented steps to reproduce it.  And your answer to it: well, just don’t do it.  That’s not a valid response to a bug report.
Unless there’s some new convincing argument, I must conclude that borsh 1.x is in state of perpetual brokenness poorly suited for no_std environments and that I should stick to borsh 0.10.

Well, i've found an example pascal_string crate/repo. It doesn't have no_std at all.
Can you post a link to the problematic repo?
Your example
#![no_std]
extern crate alloc;

struct PascalString(alloc::vec::Vec<u8>);

impl borsh::BorshSerialize for PascalString {
    fn serialize<W: borsh::nostd_io::Write>(&self, writer: &mut W) -> borsh::nostd_io::Result<()> {
        writer.write_all(&[self.0.len() as u8])?;
        writer.write_all(&self.0)
    }
}

attempts to assert, that the pascal-str doesn't use anything out of std , that's why it can work in both std and no_std.

But let’s now say someone uses it in std application like so

Is it supposed to still be using borsh::nostd_io in an std application?

Imo, this is what we need (haven't checked this specific code, might need some adjustments):
#[cfg(feature = "std")]      use std::io as io_impl;
#[cfg(not(feature = "std"))] mod nostd_io;
#[cfg(not(feature = "std"))] use nostd_io as io_impl;

pub mod io {
    pub use crate::io_impl::{Error, ErrorKind, Read, Result, Write};
}

That is:

given that borsh is io-polymorphic, clearly it should provide users with the ability to write io-polymorphic code as well.
borsh::io feels like the right name here. This is not the general "maybestd", io::Error is the only thing we need here (and we should commit to not needing anything else from the std)
we should enforce that the interface between two io modules is compatible, using a single pub use feels like a good way to do that.
it's unclear whether borsh::io should be absolutely minimal and strictly restricted to what's needed by borsh, or whether it should be a duck-type replacement for std::io. Eg, it unclear whether borsh::io should contain a version of std::io::copy -- some user code might want to call borsh::io::copy and have that work in std and non-std. I think it makes sense to expose what's already exposed. That is, neither to be absolutely minimal, nor to try to expose everything.

core::io feels like a common problem which should have solutions in the ecosystem, but quick googling didn't reveal any existing patterns. I think it is plausible that in the distant future, std::io::Error  is  moved to core, or some canonical core_io crate emerges. We should be able to take advantage of those by adding a new feature flag, so there's no compatibility hazard here.
In terms of diff for the current PR, rename&flatten borsh::maybestd::io to just borsh::io.

The only downside of using borsh::io is that inattentive folks can randomly use items from this completion instead of std::io.
@mina86 has so far ignored the question to the pascal-str example, which was the cornerstone reason, backing the push for the change done by this pr:

@mina86 has so far ignored the https://github.com/near/borsh-rs/pull/212#issuecomment-1712513408 to the pascal-str example, which was the cornerstone reason, backing the push for the change done by this pr:

Because it wasn’t a cornerstone reason.  It was a quick example I came up with on the spot.  The cornerstone reason was that there was no consistent way to name types used in the API which in turn resulted in build breaking when std feature was enabled.
I don’t care if it’s borsh::io, borsh::maybestd::io or even borsh::byćmożestd::io.  So long as the same path works regardless of what features are enabled.  And once that name is provided, pascal-str is supposed to use that.

@mina86 well, alternatively, u can use 4 lines to dispatch on an std feature, or byćmożestd feature or whatever.
 #[cfg(feature = "std")]
 use std::io;

 #[cfg(not(feature = "std"))]
 use borsh::nostd_io as io;

It's explicit and prevents you from importing borsh::io all over the place, whether you need it or not.
std is important enough to be worth a line in Cargo.toml, and, as far as i understood, the pascal-str did intend to use two different providers of io, while using borsh, depending on whether it's std or not.
5 lines in client with explicit stating of what's used when and what's not vs 500 lines change in the subj repo is a good tradeoff imo.

std is important enough to be worth a line in Cargo.toml

No, it’s not.  I’m writing a no_std library.  I don’t care about std.  I wrote code which works with borsh with default features disabled.  And then, when someone uses my library, the code breaks because borsh’s std feature was enabled.  That a bug in borsh.

as far as i understood, the pascal-str did intend to use two different providers of io, while using borsh, depending on whether it's std or not.

Yes, depending on whether borsh is std or not.  And since borsh changes types depending on whether it’s std or not, it’s borsh’ responsibility to provide a stable API for types it uses.
Imagine borsh had a uk feature and when it’s enabled rather than BorshSerialise and BorshDeserialise it would define BorshSerialize and BorshDeserialize.  That would be extremely bad API.  Situation with Read, Write etc. is the same.

5 lines in client with explicit stating of what's used when and what's not vs 500 lines change in the subj repo is a good tradeoff imo.

I can make this a ‘5 line’ change in borsh if you prefer:
+#[cfg(feature = "std")]
+pub mod io {
+    pub use std::io::{Read, Write, Error, ErrorKind, Result};
+}
+
+#[cfg(not(feature = "std"))]
+pub use nostd_io as io;

I made it 500 to simplify other things in the code.  This isn’t a valid argument.
Not to mention that even if this is a 5 lines in client library, it’s 5 lines in every library and additional changes in every crate that transitively depends on it.


Add public sdkVersion property to PluginStore

This PR gives runtime consumers of PluginStore the ability to determine the version of @openshift/dynamic-plugin-sdk package that was used in their build.
For example, host applications can print the plugin SDK version at startup for better diagnostics.

/lgtm


Added g3log_tracelogging sink

Kjell,
I've created the first cut of g3log_tracelogging.  I don't have unit tests or CI yet as I'm not yet sure how to set them up but I was able to set up a simple example and I verified that it works as expected.  I'll be reading up on ctest, gtest and Windows CI over the next few days to get there but I wanted to submit this as it is now so you could give me any suggestions you have.  I am very open to making changes and would appreciate any guidance you can give me.
Jeff

I just realized that the last two changes I made didn't get into this pull request.  I'm not sure it's possible to update the pull request.  Is the best thing for me to do to close the pull request and resubmit the correct pull request?

If you have committed your changes and pushed them to your repo, to the correct branch then they will show up on the PR (your branch towards master on /KjellKod/g3sinks)
Sent from my iPhone

On Apr 7, 2019, at 10:11 AM, jjhegedus<EMAIL_ADDRESS>wrote:
I just realized that the last two changes I made didn't get into this pull request. I'm not sure it's possible to update the pull request. Is the best thing for me to do to close the pull request and resubmit the correct pull request?
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

I just pushed them to my repo on the correct branch.  Sorry for my ignorance and thanks again for you help.

As I continue to learn more about CMake, I think I'd like to improve this and submit a new pull request at a later date.

sounds good! thanks for thorough diligence!


return a deep copy of artifact from artigact-cache

Closes: Issue #884

Caused  pickle error:
TypeError: cannot pickle 'module' object
Closing for now.


HIVE-27483: HMS add_partitions audit log adds table name and metrics

What changes were proposed in this pull request?
Add table name to audit log and metrics.
Why are the changes needed?
We are missing the table name and metrics related to add_partitions RPC.
Does this PR introduce any user-facing change?
No
Is the change a dependency upgrade?
No
How was this patch tested?

I do not find test for the audit logs, maybe I missed it, but if it possible could you please check if there is any test around it?

Yes, we are missing the audit log test.

Also please move the jira ticket into the correct state.

Updated.

@nrg4878 Could you please check this PR?

Fixed by https://github.com/apache/hive/pull/4480
Thanks for your review @aturoczy


MM-10888 Hide parts of combined system messages that don't affect the current user

The redux changes filter out combined system messages that don't relate to the current user while this makes combined system messages for the current user only show the parts that affect the current user.
User adding and removing people:

User being added and removed:

Redux PR: https://github.com/mattermost/mattermost-redux/pull/532
Ticket Link
https://mattermost.atlassian.net/browse/MM-10888

@cpanato This is waiting on the Redux PR, but since the Redux PR only has one commit, this can probably be merged once that PR is without updating the package.json.


Commands like ENTER, SPACE, q have no effect

./less file.txt
Where file.txt is an existing file.
Pressing ENTER, SPACE or q has no effect

wrong repo


Emphasize role of applied filters in report subscriptions

Given that a common end user expectation is that a report subscription arriving in their email inbox will reflect any filters applied to the report prior to creating the subscription, I'd recommend that the information on this lack of functionality be mentioned much more prominently on this page.

Document Details
⚠ Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

ID: e35489ab-e39e-b7be-588d-38f26ccf5442
Version Independent ID: f5145d5d-afbc-f3f0-7275-1186b60890d0
Content: Subscribe yourself to reports and dashboards - Power BI
Content Source: powerbi-docs/consumer/end-user-subscribe.md
Service: powerbi
Sub-service: pbi-explore
GitHub Login: @mihart
Microsoft Alias: mihart

Thanks for your comment, @nicheplayer. I've assigned this issue to the doc author for review.


Highlight write-in when input fails validation

Highlight write-in when input fails validation

Originally posted by @jrbarnes9 in https://github.com/ONSdigital/design-system/issues/30#issuecomment-656577491

Is there something 'more accessible' that can be done as the highlight of the input alone may not be enough for colourblind users?


WinPwn launching problem

Hello,
I run WinPwn on windows 10 pro dev : https://developer.microsoft.com/fr-fr/windows/downloads/virtual-machines/
I desactive Windows Defender and launch the ps1 but nothing append.

This is not an issue. Please check the screenshot!

The screenshot and the info you provide just say nothing. Please be more precise.

Exactly. He did not execute a function but a non existent script with .\WinPwn


Unable to start tt after install

after install TTconv on mac I got:
▶ tt
Traceback (most recent call last):
  File<EMAIL_ADDRESS>line 260, in __set_name__
    enum_member._value_ = enum_class._member_type_(*args)
                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: SccCode.__init__() takes 3 positional arguments but 4 were given

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/usr/local/bin/tt", line 5, in <module>
    from ttconv.tt import main
  File "/usr/local/lib/python3.11/site-packages/ttconv/tt.py", line 40, in <module>
    import ttconv.scc.reader as scc_reader
  File "/usr/local/lib/python3.11/site-packages/ttconv/scc/reader.py", line 35, in <module>
    from ttconv.scc.codes.attribute_codes import SccAttributeCode
  File "/usr/local/lib/python3.11/site-packages/ttconv/scc/codes/attribute_codes.py", line 37, in <module>
    class SccAttributeCode(SccCode, Enum):
  File<EMAIL_ADDRESS>line 557, in __new__
    raise exc
  File<EMAIL_ADDRESS>line 266, in __set_name__
    raise new_exc
TypeError: _value_ not set in __new__, unable to create it

This might be related to https://github.com/sandflow/ttconv/issues/386
Have you tried installing from source?

Closed by https://github.com/sandflow/ttconv/releases/tag/1.0.6


Binary Sensor Template: Add icon_template and entity_picture_template support

Description:
Add support for icon_template and entity_picture_template support.  Code was pulled from sensor.template and tweak to make work.
Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.github.io#4373
Example entry for configuration.yaml (if applicable):
binary_sensor:
  - platform: template
    sensors:
      alarm_chime:
        friendly_name: 'Alarm Chime'
        value_template: "{{ is_state_attr('alarm_control_panel.alarm_panel', 'chime', true) }}"
        icon_template: >-
            {% if is_state_attr('alarm_control_panel.alarm_panel', 'chime', true) %}
              mdi:bell-ring
            {% else %}
              mdi:bell-off
            {% endif %}

Checklist:

[ ] The code change is tested and works locally.

If user exposed functionality or configuration variables are added/changed:

[ ] Documentation added/updated in home-assistant.github.io

If the code communicates with devices, web services, or third-party tools:

[ ] Local tests with tox run successfully. Your PR cannot be merged unless tests pass
[ ] New dependencies have been added to the REQUIREMENTS variable (example).
[ ] New dependencies are only imported inside functions that use them (example).
[ ] New dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
[ ] New files were added to .coveragerc.

If the code does not interact with devices:

[ ] Local tests with tox run successfully. Your PR cannot be merged unless tests pass
[ ] Tests have been added to verify that the new code works.

Closeing this and moving to #12158 as I had to re-create my branch to remove the super calls.


rclone mount handling incorrect remote path

When filing an issue, please include the following information if possible as well as a description of the problem.  Make sure you test with the latest beta of rclone.
If you've just got a question then please use the rclone forum instead of filing an issue.

What is your rclone version (eg output from rclone -V)
rclone v1.35-DEV

Which OS you are using and how many bits (eg Windows 7, 64 bit)
FreeBSD

Which cloud storage system are you using? (eg Google Drive)
Google Drive

The command you were trying to run (eg rclone copy /tmp remote:tmp)
rclone mount foo bar

IIUC, the correct syntax should be rclone mount foo: bar. rclone didn't reject my incorrect remote name.
Instead, rclone still try to mount it (according to mount and df command) but the bar folder is still empty.

If you don't have a : in your remote name, it refers to a local path
rclone mount foo bar

This will cause rclone to mount the directory in the current directory called "foo" onto the directory "bar".  Due to the way rclone works, if "foo" doesn't exist, then it doesn't matter - it will be created if you copy files into the mount.
So that is expected behaviour.
However I could make rclone mount check the directory exists which might make more sense - it would then exit with an error "directory not found".

Ah, this makes sense. I am okay to keep current behavior. Feel free to close this issue if you don't plan to change the behavior.

OK I'll leave rclone how it is until I get more requests!
Thanks


conda package for h2o4gpu

Would be great to have a conda package for h2o4gpu. Appears one was added for h2o recently.

@hemenkapadia , can you clarify if the conda package you have created will be built for open-source h2o4gpu as well?  I think that's the goal, right?

Yes, it will.

PR #671 created to merge conda build recipe with master. Local conda package can be created using make docker-buildtill internal conda server is updated to release the build conda packages on h2oai conda channel.

Taking the strategy as implemented in caffe2 (https://anaconda.org/caffe2/caffe2/files) to provide cuda build variants for h2o4gpu. Assuming that package name will be self explanatory for a user installing it.

Work completed. Awaiting PR merge to master.

@jakirkham , yes once the PR is merged and a release build is run, the packages will be published to anaconda cloud too at https://anaconda.org/h2oai

h2o4gpu packages are now available in h2oai channel on anaconda cloud. https://anaconda.org/h2oai.
Closing the issue.


Allow for User Specified JSON Types in TypeScript

Problem
When a model is created with a JSON field, the generated TypeScript returns the type Prisma.JsonArray or Prisma.Json. As a dev, I'd like to be able to specify my own type at the ORM level rather than type cast manually every time I interact with that model through Prisma.
model Post {
  id Int @id @defualt(autoincrement())
  data Json
}

Suggested solution
new PrismaClient<{ Post: { data: PostData } }>()

Alternatives
model Post {
  id Int @id @defualt(autoincrement())
  data Json @typescript(PostData)
}

Other Context
I know the above answer is contrived, but suffice it to say it's reasonable to want typing for JSON fields! Thanks for the awesome library.

Would this be covered by something like https://github.com/prisma/prisma/issues/3219 as well?

Same issue, different solutions. I'll close and suggest the changes there.


Every request is Pending

Like the title suggests, every request shows as Pending and never updates the status to either Success or Failed. The app is working fine, data is populating as it should.
Has anyone else had this problem? I'm running Pulse 3.2.1 on an iOS 16.0 sim

Potentially related: https://github.com/kean/Pulse/issues/113

Closing since #113 covers it


Errors about the installation of patchwork-plusplus in your projects

Thanks for your share of impressive work! When I install the patchwork-plusplus, some errors happen when I only run the pip install .in patchwork-plusplus directory from the original project. I just wonder if you have install the Open3D in C++ version as Prerequisite packages for patchwork-plusplus, because I just skip them for simplification.

I realized that the reason may be that the cmake version on my desktop is too slow with cmake 3.10.2. I would greatly appreciate that if you could tell me which version of cmake adopted by you.

Thanks for your early reply, and could you please tell me the version of Cmake on your desktop?


Add info from one-pager

add text from one-pager in readme.md which will also render to homepage
image wasn't rendering well b/c it auto-expanded to fill whole page and was giant.  Decided to render less cool image in mermaid instead.  Text/color isn't ideal b/c you can't change text color in mermaid only the css as a whole. boo. So had to use colors from our theme that you could read with white, black and color backgrounds.
also added minutes page for board meetings.

This is a non-normative change.

Thank you for adding this @e-lo !
The one part I am worried about is the mermaid diagram. It may beg more questions than it answer at this point. I'd rather leave this off for now and work toward more complete visuals in the best practices documentation.

@jlstpaul - i updated the readme in the PR to not have mermaid diagram.


Loading multiple separate dataframes with the same metadata

The metadata provides support for having multiple tables in one schema -- how do I connect multiple dataframes to metadata with multiple tables?

is it not possible?

The multi-table scenario is meant for database engines where a database holds a collection of tables.  In the case of pandas dataframes, the "connection" we get to the factory to create the PandasReader is a single table.  Since we don't pass in any table name information, the PandasReader just grabs the first table in the metadata.
We could fix this by allowing people to pass in a dictionary of pandas dataframes keyed by a desired table name.  For example:
from snsql import from_connection, Privacy
import pandas as pd

classes = pd.read_csv('classes.csv)
students = pd.read_csv('students.csv')
meta = '../education.yaml'

reader = from_connection({'students': students, 'classes', classes}, meta=meta, privacy=Privacy(epsilon=1.0))

Another option would be to finally implement the SQLiteReader and provide a utility to populate a new SQLite database from a collection of pandas dataframes.  That would give better performance for people doing lots of queries, but would add an extra step for people who want to just quickly query a couple of pandas dataframes.

Ah, yep, I see.

This has been added to smartnoise-sql==1.0.0.  Multiple pandas datafranes can be passed in with a dictionary.


Consistent focus state

Some of the libraries used in the app override the focus state. It would be better from an accessibility point-of-view to consistently use the yellow-rectangle focus style from the GDS stylesheet.

Not perfect in every case, but I think it's acceptable

confirm good to go


Produce proc_macro2::TokenStream instead of quote::Tokens

Do these need to be different types? Conceptually they are both Vec<TokenTree>. Would it be possible for quote! to produce proc_macro2::TokenStream (and eventually, proc_macro::TokenStream)?

The biggest disadvantage of this is that TokenStream doesn't have near as large of an API surface as Vec<TokenTree> - for example it isn't possible to .push() a new TokenTree onto the end at the moment. We'd have to change the ToTokens API a lot, and do some tricks to use Vec<TokenTree> while building up the result of quote!(), in order to avoid the cost of constantly destructing and re-building TokenStream objects.
I think for now using a different type is probably our best option.


chaos(engine): fix time shift case, use correct string type

What problem does this PR solve?

Issue Number: ref #5640
and  #6143
What is changed and how it works?
v1alpha1.TimeChaosSpec.Value: ReadString: expects " or n, but found 5

Value should be a string type.
Use random-max-percent to add some randomness.

Check List
Tests

Unit test
Integration test

Questions
Will it cause performance regression or break compatibility?
Do you need to update user documentation, design documentation or monitoring documentation?
Release note
None

Codecov Report

Merging #6185 (43dfe54) into master (ef587e6) will decrease coverage by 0.3796%.
The diff coverage is 72.5168%.

Flag
Coverage Δ

cdc
63.8068% <74.2796%> (-0.8778%)
:arrow_down:

dm
51.8748% <48.4127%> (-0.0143%)
:arrow_down:

engine
62.9144% <77.5362%> (+0.3805%)
:arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.
@@               Coverage Diff                @@
##             master      #6185        +/-   ##
================================================
- Coverage   58.4193%   58.0397%   -0.3797%
================================================
  Files           711        706         -5
  Lines         84205      83305       -900
================================================
- Hits          49192      48350       -842
- Misses        30540      30602        +62
+ Partials       4473       4353       -120


Fixes #2054 - Add db indexes to all search params

(Holding this in draft until the v41 migration is complete in https://github.com/medplum/medplum/pull/2037)

Coverage: 94.272%. Remained the same when pulling 065ab960231c450410f751ee1d70f641b563bf8f on cody-more-search-indexes into fbbbc390274a5e4e7ec01635d4f8a7679e837f46 on main.

I recently read this article, which touches on some performance gotchas that we might run into with this strategy: mainly, that index merges are possibly not as well-supported by Postgres as they are by MySQL.  The article was using a COUNT(*) query though, which is known to be slower in Postgres, so I'm unsure how much of their findings to attribute to that vs. the query planner

@mattwiller That is interesting, thanks for sharing.  It is an unfortunate downside of the flexible FHIR search model that it would be difficult to preempt all of the composite indexes.

We should continue to track common query patterns, and strategically add composite indexes
Considering that most queries are for compartment + one or more search params, it's possible that the better model would be that most of these are composite searches for ("compartment", "<search param>"). :/

Update: I'm running benchmarks to determine whether this will meaningfully improve performance.

Query:
SELECT
  "Communication"."id",
  "Communication"."content"
FROM
  "Communication"
WHERE
  (
    "Communication"."deleted" = $1
    AND (
      "Communication"."compartments" IS NOT NULL
      AND "Communication"."compartments" && ARRAY[$2] :: UUID[]
    )
  )
ORDER BY
  "Communication"."received" DESC
LIMIT ?
OFFSET ?

Before index: 25-35ms
After index: 1-3ms
Unfortunately the COUNT query is still slow :(

That's a great result! 🎉


Do you have a roadmap?

Do you have a roadmap for this implementation?
For example:

implement the /.well-known/openid-configuration handler
implement a token-introspection handler
...

The short answer is there is no roadmap for new features at the moment. This library does exactly what I need it to do. I've purposely kept it as simple as possible.
If there are missing features you would like to see please let me know about them!

Aah, I see you only implemented the OIDC core RFC. If that is your only goal, then I'm all OK with that.
I wanted to have the OIDC Discovery and Dynamic Registration too in my setup.

@leroy0211 Sorry for taking awhile to respond to this. I'll take a more in-depth look at these. I do believe these features would be great additions to this library. Thanks!

Hello :)
I wanted to know if there were plans to add the /token/introspection and the /userinfo endpoint ?
Explaination:
https://connect2id.com/products/server/docs/api/token-introspection
https://connect2id.com/products/server/docs/api/userinfo

I can second that it would be a good enhancement.
I had to implement it on top of open-id-connect using one pull request from the underlaying League OAuth2 server, when I implemented (OpenID Connect for EGroupware)[https://github.com/EGroupware/openid].
I'm happy to prepare both a pull request, if I know it will be merged.
Ralf

@stadja @ralfbecker I would definitely be interested in any improvements to this library you would be willing to contribute. My time is very limited at the moment but as soon as I can i'll review the token-introspection and usreinfo links provided above.

I added now a couple more enhancements to my OpenID Connect server for EGroupware:

[x] fix League OAuth2 server to support multiple response_type(s), currently it neither splits response_type by space, nor does it send responses for more then one grant, see response in this ticket
[x] support response_type "id_token" or "token id_token" containing just an id_token (JWT) or additional to access_token an id_token
[x] support hyprid flow / response_type contains additional "code" to also return an auth_code
[x] add additional OpenID Connect standard scopes: profile, address, phone

Ralf


claim wc

Oh, I am sorry, I didn't notice claims as issues and implemented wc in https://github.com/vlang/coreutils/pull/52.
You are free to close the PR.

Closed by #52


how to use the swc plugin with Rspack

rt
I want to use a third party swc plugin in rspack How to configure

not supported yet, we may consider support it in the future

A swc-loader with native plugin may be used as a current workaround.

swc wasm plugin is quite experimental. We have no plan for it now.


cmd/go: go get should not add a dependency to go.mod

go get is not the right command for adding a dependency to the current module.  We have a command for dealing with modules, and it's go mod, that's the command that should be used to add dependencies.
This is the first paragraph of help on go get:

Get downloads the packages named by the import paths, along with their dependencies. It then installs the named packages, like 'go install'.

None of that says "add the library you specified to the dependencies of the current project".  And why would it?  it's for getting code from somewhere else and bringing it to this machine.
  Your go.mod now has
golang.org/x/tools v0.0.0-20180911133044-677d2ff680c1 // indirect

This UX is extremely confusing for the ~2 million people who used go before modules.  It's also way too implicit and magic regardless of the experience.  Adding a dependency to your project should be an explicit action:
go mod add golang.org/x/tools/cmd/goimports

What did you do?
from inside a module directory
$ ls
go.mod
$ go get github.com/natefinch/lumberjack
go: finding github.com/natefinch/lumberjack latest
go: downloading github.com/natefinch/lumberjack v0.0.0-20180817145747-7d6a1875575e
~/dev/test$ more go.mod
module app

require github.com/natefinch/lumberjack v0.0.0-20180817145747-7d6a1875575e // indirect

What did you expect to see?
No change to my go.mod
What did you see instead?
go get added a library to my go.mod file.
System details
go version go1.11 darwin/amd64
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/finchnat/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/finchnat"
GOPROXY=""
GORACE=""
GOROOT="/Users/finchnat/sdk/go1.11"
GOTMPDIR=""
GOTOOLDIR="/Users/finchnat/sdk/go1.11/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
GOROOT/bin/go version: go version go1.11 darwin/amd64
GOROOT/bin/go tool compile -V: compile version go1.11
uname -v: Darwin Kernel Version 17.7.0: Thu Jun 21 22:53:14 PDT 2018; root:xnu-4570.71.2~1/RELEASE_X86_64
ProductName:	Mac OS X
ProductVersion:	10.13.6
BuildVersion:	17G65
lldb --version: lldb-9<IP_ADDRESS>
  Swift-4.1

From the bottom of go help get:
...

This text describes the behavior of get when using GOPATH
to manage source code and dependencies.
If instead the go command is running in module-aware mode,
the details of get's flags and effects change, as does 'go help get'.
See 'go help modules' and 'go help module-get'.

From go help module-get:
The 'go get' command changes behavior depending on whether the
go command is running in module-aware mode or legacy GOPATH mode.
This help text, accessible as 'go help module-get' even in legacy GOPATH mode,
describes 'go get' as it operates in module-aware mode.

...

@natefinch I think the documentation on this covers the points you raise, but please do say if you think it could be better signposted.
cc @bcmills

It's not about the documentation.  It's a complete change to what go get has meant for the last 6+ years.  Which has meant "download and compile some code so I have it locally".   That's still a useful command to have in the universe of go modules.
Changing what an established command does, in such an important way, is a really bad idea.
I think it's pretty clear, if I go get golang.org/x/tools/cmd/goimports ... what I mean is "download and compile goimports, and put it somewhere for general use".  The fact that this command also stealthfully adds golang.org/x/tools/ to my current directory's dependencies is surprising and extremely unwelcome.
Like I said in the bug report... we have a command for dealing with modules. Why would we not just add this functionality there?  go mod add github.com/... is perfectly clear, and wouldn't clash with current usage.

if I go get golang.org/x/tools/cmd/goimports ... what I mean is "download and compile goimports, and put it somewhere for general use".
I think this is; https://github.com/golang/go/issues/25922 and https://github.com/golang/go/issues/24250

I agree with this. I've already been surprised three separate times now by running go get and having tools unexpectedly added to my go.mod, or having a new go.mod file dropped into a directory where I didn't intend.
Before modules, go get was used for two pretty distinct use cases: (1) downloading tools and (2) downloading libraries. It seems much, much less surprising and user-friendly for post-modules go get to specialize on use case (1) and leave (2) for other tools. In my experience with modules so far, I've handled (2) by simply adding new imports and running go test/go build.

I think it's pretty clear, if I go get golang.org/x/tools/cmd/goimports ... what I mean is "download and compile goimports, and put it somewhere for general use".

In a GOPATH world I would agree. But we're talking a modules world here. From my perspective I'm fully anticipating any go command to do something with my go.mod.
Put another way (and this is where https://github.com/golang/go/issues/24250 comes in), I would expect, in a modules world, to have to do something "special" in order to performa a "global" install (by definition outside the context of my go.mod)

I was also surprised by this behavior. The suggestion that go get will have a different meaning in a "modules world" doesn't solve the use case of go get golang.org/x/tools/cmd/goimports or installing any other tool. Do I need to change directories every time I need to install a new tool? What if a go.mod is created anywhere I am because in the modules world any place is a valid place for a Go project?
Regardless of these examples, what bothers me the most is that we're using the same command that has existed for several years to do a different thing. Are we all supposed to accept that every tutorial, every doc, everything that has ever been said about go get is not valid anymore?
The global place for installing any library or tool still exists, so why do we need to do something special to get back the behavior that already exists?

Have to chime in that that I agree with @vdemario here.  Using go get for installing tools has been the standard practice since go get was created.  Changing that semantic is extremely problematic from a usability perspective, especially since the Go tooling currently lacks a command equivalent to "download this Go library, compile it, and install it somewhere I can use, but do not add it to my current project".
We're adding entirely new behavior via the go mod command.  It seems to make the most sense to have explicitly adding a dependency to be a subset of that command, rather than hijacking an existing command and completely obliterating one of the two most common ways that command is currently used.

@vdemario please see, as linked above, https://github.com/golang/go/issues/24250

That issue clarifies this one even more.  Prior to Go 1.11, the semantics for go get were identical to go install, except that it pulled the code from a remote repository before running go install on them.  The module functionality in Go 1.11 has completely abandoned the go install piece (actually, it's still there, but it's mostly an anachronism at this point) because go get has been hijacked for adding a dependency to a module.
And that's a bit of a problem, since go get was validly used just as commonly for the go install functionality as it was for the abstraction of git clone.  Abandoning the install functionality is extremely jarring and leads to some very unexpected side-effects, like the necessity of running go mod tidy after installing a tool that's not also a dependency, and as @vdemario just pointed out, the complete inability to install tools unless you're currently in a module (which you then need to tidy).

@kaedys

The module functionality in Go 1.11 has completely abandoned the go install piece (actually, it's still there, but it's mostly an anachronism at this point) because go get has been hijacked for adding a dependency to a module.

And that's a bit of a problem, since go get was validly used just as commonly for the go install functionality as it was for the abstraction of git clone. Abandoning the install functionality is extremely jarring and leads to some very unexpected side-effects, like the necessity of running go mod tidy after installing a tool that's not also a dependency

go get does still perform an install. Given:
export GOPATH=$(mktemp -d)
cd $(mktemp -d)
mkdir hello
cd hello
go mod init example.com/hello
export GOBIN=$PWD/.bin
go get golang.org/x/tools/cmd/stringer

Then:
$ ls $GOBIN
stringer

like the necessity of running go mod tidy after installing a tool that's not also a dependency, and as @vdemario just pointed out, the complete inability to install tools unless you're currently in a module (which you then need to tidy).

This falls under the umbrella of https://github.com/golang/go/issues/24250.
As @komuw linked above, for discussion on the current "best practice" for defining tool dependencies, please see https://github.com/golang/go/issues/25922

But it's not about tool dependencies.  It's about tools you want to install on a global level, unrelated to the current module.  That's the issue at the root of this, that under the Go module framework, go get is inherently and unavoidably linked to the module you are currently operating on (and completely fails if you're not on a module).  There's no reason to abandon the ability to install tools unrelated to the current module (or outside the context of a module), and there's no reason to assume that every package retrieved is necessarily a dependency of the current module given the pre-1.11 semantics and usage of go get.

It's about tools you want to install on a global level

Indeed, that's why I linked to https://github.com/golang/go/issues/24250.
Just to emphasise what @rsc said in https://github.com/golang/go/issues/24250#issuecomment-377553022:

$ vgo get<EMAIL_ADDRESS>cannot determine module root; please create a go.mod file there

This clearly must work eventually.

(we can now substitute go get for vgo get)
So there is agreement that this "global" concept indeed very much has merit. It's just that this didn't make Go 1.11.

There's no reason to abandon the ability to install tools unrelated to the current module (or outside the context of a module), and there's no reason to assume that every package retrieved is necessarily a dependency of the current module, especially given the pre-1.11 semantics and usage of go get.

Hopefully the link above covers this point?

Why aren't there go mod add and go mod update commands?  That would seem to be the obvious place to go looking for the functionality of adding or updating a dependency.

@natefinch

We have a command for dealing with modules, and it's go mod, that's the command that should be used to add dependencies.

You are mistaken. The go mod command is for module-specific operations, not all operations “dealing with modules”. As its documentation says, “support for modules is built into all the go commands, not just go mod. For example, day-to-day adding, removing, upgrading, and downgrading of dependencies should be done using go get.”
go get in GOPATH mode already performs a grab-bag of tasks: it “downloads the packages named by the import paths, along with their dependencies. It then installs the named packages, like go install.” go get has the same behavior in module mode, except that it downloads entire modules and it does not yet work outside of a module (that's #24250).
In my opinion, it is unfortunate that go get conflates downloading packages with installing binaries, but we're essentially stuck with it: we need to maintain compatibility with existing, documented command lines, and existing command lines don't only use go get to install tools — they also use it to download package dependencies.

go get foo/bar/baz explicitly tells the go tool "ignore the directory that I'm in. Instead, find this package, make sure it and its dependencies are downloaded locally, build it, and install it."

But go get -u foo/bar/baz upgrades arbitrarily many dependencies: it may affect the current directory, depending on what else is involved.
As weird as it is for go get of a tool to update the current module but install the tool globally, it seems even weirder for go get -u not to upgrade dependencies, or for go get -u to affect the module where go get would not.

It seems counter-intuitive that a command that already does too much is getting new responsibilities and side effects (such as updating go.mod).

Every command that loads packages updates go.mod, and go get -u already adds and upgrades dependencies even in GOPATH mode. We're not adding new responsibilities to go get so much as to go globally.
(We did add one new responsibility to go get: it can now downgrade dependencies, whereas the go command couldn't downgrade dependencies at all before.)

it seems even weirder for go get -u not to upgrade dependencies, or for go get -u to affect the module where go get would not

I feel like this is a disconnect between people who use a single gopath for everything (i.e. google) and people who exclusively use vendored dependencies.
Every serious public project for a long time has used vendored dependencies.  go get -u would never update any of these dependencies, because they all use vendoring.

IMO go get -u <some library> just doesn't really make any sense in the modules world.  It's a holdover from gopath.  It works for binaries because it updates the binary in gobin.
I'd really prefer if the commands to update dependencies were explicit and obvious named.   go update foo/bar or go add foo/bar.
Why hang on to this fairly vague word "get" when we're talking about actions that never existed before?  go get has baggage that we can leave behind.

I feel like this is a disconnect between people who use a single gopath for everything (i.e. google) and people who exclusively use vendored dependencies.

Most Google-internal users pretty much aren't using GOPATH (or the go tool) at all. This isn't a Google-vs.-outside issue so much as a hobbyist-vs.-enterprise one.

Every serious public project for a long time has used vendored dependencies. go get -u would never update any of these dependencies, because they all use vendoring.

Yes, we clearly need to work on the migration path for vendoring. That's one of my top priorities for 1.12.

I completely agree that get is a weird holdover from the pre-modules world, but I don't think we can leave that baggage behind. Go takes a very serious approach to backward-compatibility, and breaking install instructions that use go get -u would be a pretty big incompatibility.
That said, maybe it's worth leaving the get baggage behind when we're talking about installing tools: for example, go install as it stands is very nearly useless in module mode. Perhaps go install pkg@version should install the named version of the tool without updating the local go.mod.

go get -u as instructions to install a library have been invalid ever since the widespread use of the vendor directory.  I'd be interested to see how many widely used libraries actually put that in their readme.

This isn't a Google-vs.-outside issue so much as a hobbyist-vs.-enterprise one.

Sorry!  I almost deleted the google bit there. definitely was not trying to make this a google vs. the outside world.

I'd be interested to see how many widely used libraries actually put that in their readme.

I'd be curious to see that too!
For a couple of other data points:

Google hit # 4 for update go dependencies is this StackOverflow question, for which the “accepted” answer is go get -u — and none of the other answers mention vendoring at all.

(The first three hits are dep, a blog post about dep, and #13332, so that's the first hit that actually mentions how to use the go tool itself.)

I found almost 500 hits for go get -u in Google's codebase (which includes a mix of scripts, HTML documentation, and READMEs).

(To be clear, none of this contradicts your point that serious public projects use vendoring. Ideally, we want a solution that works for serious projects and hobbyists.)

To be fair, that question was answered in 2012 :)

Here are three quick current examples of instructions using go get -u:
go-redis (https://github.com/go-redis/redis)

Install:
go get -u github.com/go-redis/redis

grpc (https://grpc.io/docs/quickstart/go.html)

Use the following command to install gRPC.
$ go get -u google.golang.org/grpc

aws-sdk (https://docs.aws.amazon.com/sdk-for-go/v1/developer-guide/setting-up.html)

Install the AWS SDK for Go
To install the SDK and its dependencies, run the following Go command.
go get -u github.com/aws/aws-sdk-go/...

Those might not be perfect examples (aws-sdk also includes commands, I think), or maybe turns out to be uncommon, but that was after a very brief search.
In any event, it still might be smart to change the current approach for what go get means, or to add go add or go update. Just wanted to provide some quick examples.

As weird as it is for go get of a tool to update the current module but install the tool globally,

I actually think the absence of a "default module-local install location" falls into the same category of "missing from Go 1.11" as https://github.com/golang/go/issues/24250. Let me try to explain by covering my "workflow".
The tools I use on a day-to-day basis fall into two categories:

Tools I want to be controlled by a project's go.mod
Tools that I need globally

Dealing with these in reverse order.
Category 2: I think it's clear that with Go 1.11, there is a "gap" here and that this is covered by https://github.com/golang/go/issues/24250. Per the detail in that discussion, there are open questions on how to handle multiple versions, where the installed binaries should be put etc, but it all falls under that issue.
Category 1: by far the largest category of tools for me, made up largely of code generators that I use with go generate and the like. I absolutely want these to be version controlled. And I don't want to be using (via my PATH) a "global" install of such a tool, even if the version just happens to match at that point in time. But both go get and go install currently (i.e. Go 1.11) have a target of $GOPATH/bin (ignoring multi-element `GOPATH values for now).
Hence the workflow I have effectively adopted is to create a module-local install target:
# create a new module
cd $(mktemp -d)
mkdir hello
cd hello
go mod init example.com/hello

# set GOBIN as a module-local install target
export GOBIN=$PWD/.bin

# update my PATH accordingly (I actually use https://github.com/cxreg/smartcd for this)
export PATH=$GOBIN:$PATH

# add a tool dependency (by definition, category 1 tool)
# following "best practice" laid out in https://github.com/golang/go/issues/25922#issuecomment-412992431
cat <<EOD > tools.go
// +build tools

package tools

import (
        _ "golang.org/x/tools/cmd/stringer"
)
EOD

# install the tool
go install golang.org/x/tools/cmd/stringer

# verify we are using the module-local binary
which stringer

# which gives something like:
# /tmp/tmp.Hh0BNOF6k2/hello/.bin/stringer

As mentioned, one of the disconnects (in this thread) is that a go get or go install in a module context modifies the "local" go.mod but installs "globally". This is, as @bcmills put it, "weird". But is to my mind a gap in Go 1.11, just as there not being a "global" tool install is a gap (i.e. https://github.com/golang/go/issues/24250). Remember, module support is experimental and almost by definition incomplete.
go run is a potential alternative to the "local" install here (and a very attractive one to my mind), but it is a non-starter unless we can find a way to address https://github.com/golang/go/issues/25416.
There could be some convention that a .bin/ directory, alongside a go.mod, is the target for go get and go install (of main packages) for "local" installs? But this wouldn't obviate the need for everyone to update their PATH and indeed .gitignore the .bin directory for every module they work on.
And I'm sure there are many other potential solutions.
But I thought it worth pointing out that I see this as a gap, as opposed to a final decision on workflow.
@bcmills I don't know what your thoughts on this are, and whether it's worth creating a separate issue for the "local" install target/run solution?

A note on local binaries,  https://github.com/twitchtv/retool does exactly what you want, @myitcv ... the only drawback being that you need to run them by running retool do <binary>.  So like retool do dep ensure

I don't know what your thoughts on this are, and whether it's worth creating a separate issue for the "local" install target/run solution?

Yes, please file a separate issue.
(Let's try to stick to one decision per issue so we don't lose track when issues are closed. As I see it, the decision for this issue is “should go get update go.mod?”)

one more thing I just encountered that is related.
running go build foo/bar/baz in a module directory ... adds foo/bar/baz to the go.mod.   That seems really weird.    Now, I'll grant you, go build foo/bar/baz in a module context.... sorta doesn't make any sense... in which case maybe we just error out?  I don't really think there's any action that makes sense in a module context where foo/bar/baz is not part of the current module.

Just a quick note - as far as I've seen, it's always been clear that all of cmd/go would be adapted to work with modules. This is why something like go dep ensure would have never been a thing. To me, go mod add or go mod update fall in the same category.
I agree that the semantics of go get and go install may get a bit confusing during this gopath-to-modules transition period. Still, I don't think it's confusing enough to warrant stuffing all of these operations as sub-sub-commands under go mod. That's not a great long-term solution either, in my opinion.

@bcmills

(Let's try to stick to one decision per issue so we don't lose track when issues are closed. As I see it, the decision for this issue is “should go get update go.mod?”)

Absolutely agree. Was just using that example as a situation where I see a "gap" as opposed to a "decision" as a gentle reminder to everyone that modules are still experimental.
Just opened https://github.com/golang/go/issues/27653

I still think that overloading go get and go install is a mistake. We could just leave those as is and add more explicit commands like go add or go update.
People keep saying that module support is built into the commands and ignoring the fact that the way they're built in is the confusing part.

Regarding possibly changing the behavior of go get or introducing new commands, one consideration raised is the degree to which some or most historical install instructions can continue to work (as discussed above for this issue, and as you can see that is also a very important consideration for #24250 to make sure go get some/command always works in a module-world, although the best full solution for #24250 might need to wait until modules are on by default -- see for example https://github.com/golang/go/issues/24250#issuecomment-377553022 and  https://github.com/golang/go/issues/24250#issuecomment-411860657 comments by Russ, though of course further discussion about that aspect probably deserves to live in #24250).
Another consideration for the "should go get update go.mod?" issue here that I wanted to briefly highlight is what was termed the build "isolation rule", which was described in this blog post back in Feb:
If you are thinking about how to best modify the go command behavior (e.g., as discussed above by altering what go get does, or perhaps introduce a new go add and go update, or ___), it is worth reading that full blog post. (It is not too long, and it is the first 2/3 or so that is most relevant to the discussion here).
Some of the key snippets (though still worth reading the post itself):
https://research.swtch.com/vgo-cmd

the 'isolation rule':
The result of a build command should depend only on the source files that are its logical inputs, never on hidden state left behind by previous build commands.)
That is, what a command does in isolation—on a clean system loaded with only the relevant input source files—is what it should do all the time, no matter what else has happened on the system recently.

and:

Of course, all the [new] vgo get variants record the effect of their additions and upgrades in the go.mod file. In a sense, we've made these commands follow the isolation rule by introducing go.mod as an explicit, visible input replaces a previously implicit, hidden input: the state of the entire GOPATH.

and:

every command should have only one meaning, no matter what other commands have preceded it.

and:

Plain [pre-module] go get, without -u, violates the command isolation rule and must be fixed. [The pre-module go get has three meanings]:

If GOPATH is empty, go get rsc.io/quote downloads and builds the latest version of rsc.io/quote and its dependencies (for example, rsc.io/sampler).
If there is already a rsc.io/quote in GOPATH, from a go get last year, then the new go get builds the old version.
If rsc.io/sampler is already in GOPATH but rsc.io/quote is not, then go get downloads the latest rsc.io/quote and builds it against the old copy of rsc.io/sampler.

Overall, [pre-module] go get depends on the state of GOPATH, which breaks the command isolation rule. We need to fix that.

There are different behaviors that could be choosen while still satifisfying the "build isolation rule" when it comes to go get (including "no more go get and instead new commands X and Y" could satifisfy the "build isolation rule"), but at least wanted to highlight that blog post for people as a design consideration given that post describes some of the philosphy behind having go build by default download code as needed and some of the corresponding current changes in behavior for go get in a modules world.

@natefinch

https://github.com/twitchtv/retool does exactly what you want

Thanks. I have local scripts/hackery that also get the job done; I guess I'm pushing for the go tool to do this for me and others so that you don't need to install another tool to use tools :)

RE: isolation, I guess the jarring part for me is that, up until now, go get on a tool has had no effect on the current project or repository unless that tool is also an import dependency of that project/repo.  In the module system, go get always affects the current module, even if the user is just trying to download and install a globally-useful tool rather than an import dependency for the current module.
To an extent, that's why I favor shifting that functionality to the rather more intuitively related go install with a new flag for remote fetching.

I have never in 6 years of writing Go, run go test example.com/foo/bar.  If I want to twiddle with example.com/foo/bar before I use it, I go get it, switch to that directory, and run local commands there, because it's so much easier (like being able to look at files etc).
Literally the only time I specify an absolute package path is using go get.  Every other time, I cd to the directory and use relative package names.... because it just saves so much typing if I have to run more than one command.
In fact, this is one thing I worry about when changing go get.  it was super useful for.... getting code.   I would propose that we leave go get as it is for expressly that purpose - getting Go code onto your machine so you can look at and fiddle with it as needed (as well as a super easy way to install binaries).
Here's my workflow:

google for that package name that I can't quite remember
go to the github page, check out the readme, maybe peek at the code to make sure it looks sane
click on the godoc.org badge to get a better view of the API
run go get on my machine to get the code locally.
open that (now local) repo in my editor to look at the code / run tests etc.
go back to my project, add the import, add a little code so goimports won't take away the import ;)
rebuild my project.

Right now there's usually a 6.5 where I run dep ensure, but I'm happy to have go build do that for me.
The thing about "having modules built into the go tools" is that I expect that only packages the code actually depends on will be added to the go.mod.  Adding something my code doesn't explicitly use should only happen through an explicit command that I run that says "yes, I know I'm not actually using this, but add it to my go.mod anyway".

It should be noted that even using GO111MODULE=off go get <import path for a main package> isn't a solution, as this disables the version checking on the dependencies of that package's module. The only workarounds are to either clone the repository manually and use go install, or to navigate to an empty directory somewhere, such as using cd $(mktemp -d), initialize that as a module, and then run go get.

Here's my workflow:
1. google for that package name that I can't quite remember

2. go to the github page, check out the readme, maybe peek at the code to make sure it looks sane

3. click on the godoc.org badge to get a better view of the API

4. run `go get` on my machine to get the code locally.

5. open that (now local) repo in my editor to look at the code / run tests etc.

6. go back to my project, add the import, add a little code so goimports won't take away the import ;)

7. rebuild my project.

 Especially in large projects with a large list of dependencies.

We probably need a go mod tidy mode that can be used as a commit hook. I've filed that as #27682.

Yes, there's lots of documentation out there using go get .... if we don't muck with go get, that documentation doesn't have to be incorrect.

As previously noted, go get -u is the existing command for “update dependencies” in the non-vendored workflow. I think it's important that we not break the meaning of that command either, and its equivalent in module mode seems pretty clear.
It seems like there are three properties we would ideally like, and we can choose at most two:

go get $pkg continues to mean “install $pkg as a global tool”.
go get -u $pkgs continues to mean “upgrade the transitive dependencies of $pkgs”.
go get and go get -u differ only in whether they upgrade dependencies, including in module mode.

(1) seems more important for existing users with the vendored (enterprise) workflow.
(2) seems more important for existing users with the non-vendored (hobbyist) workflow.
(3) seems more important for new users.

@natefinch, here's my mental model for why this makes sense. I hope it will help you.
There must be some answer to the question of "which versions of which libraries are available to a build?" The answer used to be "the specific copies in GOPATH". That is, the entire GOPATH tree was in some sense equivalent to a traditional dependency manager lock file. (Tools like godeps made this more explicit, providing conversion back and forth between the exploded GOPATH and a single file recording versions.) How did we add new libraries or modify which versions were being used? go get.
In the new module world, GOPATH is gone, and the answer to which versions of which libraries are available is the go.mod file. In essence, each go.mod is now a description of a "virtual GOPATH" containing the specific modules and versions listed in the go.mod and their dependencies. We used to say that GOPATH defined the workspace; now go.mod defines the same sense of workspace, although we no longer use that term.
Just as "go get" once pulled code into, or updated code already in, the GOPATH workspace, now it does the same for code in the go.mod-defined workspace. It used to operate by writing file trees in GOPATH. Now it effects the same changes by writing lines in go.mod.
I hope that viewed this way it's clear that "go get" is doing what it has always done: set up which specific packages are going to be used by a future build.

There must be some answer to the question of "which versions of which libraries are available to a build?" The answer used to be "the specific copies in GOPATH". That is, the entire GOPATH tree was in some sense equivalent to a traditional dependency manager lock file. (Tools like godeps made this more explicit, providing conversion back and forth between the exploded GOPATH and a single file recording versions.) How did we add new libraries or modify which versions were being used? go get.

No, not really.
I mean, some people did, but as I explained in https://github.com/golang/go/issues/27643#issuecomment-420767420 any serious team-based Go workflow before modules didn't involve using go get for managing application dependencies. People used vendoring, or godep, or some other system.
In that world, the GOPATH is a handy global workspace for downloading and experimenting with libraries, scratch code, and (most importantly) installing Go tools globally.
In the module world, there is no equivalent way of saying "please install this tool globally, unrelated to the working directory I happen to be in", and attempting to use the old method has the frustrating side effect of editing the current module.

In the module world, there is no equivalent way of saying "please install this tool globally, unrelated to the working directory I happen to be in

I think that is supposed to be resolved in https://github.com/golang/go/issues/24250 and/or https://github.com/golang/go/issues/25922

I'm a little disappointed we didn't find a solution to the original author's problem here - go get does now have side effects it did not used to, that's not really debatable. One of the common use cases for Golang is to write CLI tools, and given that Golang is already installed, being able to have users do GOBIN/path/to/bin go get github.com/team/project/cmd/project@VERSION without side effects would have been a pretty useful pattern.

@natefinch this is my script workaround to the problem, if it helps - could be cleaned up and formalized:
#!/bin/bash

set -euo pipefail

if [ ! -d "${2}" ]; then
  echo "usage: ${0} path/to/go/cmd@VERSION path/to/bin" >&2
  exit 1
fi

PKG="${1}"
BIN="$(cd "${2}" && pwd)"

TMP="$(mktemp -d)"
trap 'rm -rf "${TMP}"' EXIT

cd "${TMP}"
echo "module tmp" > go.mod
GOPATH="" GO111MODULE=on GOBIN="${BIN}" go get "${PKG}"

I have been using
https://github.com/myitcv/gobin to install tools globally
On Fri, 21 Dec 2018 at 01:52, Peter Edge<EMAIL_ADDRESS>wrote:

@natefinch https://github.com/natefinch this is my script workaround to
the problem, if it helps - could be cleaned up and formalized:
#!/bin/bash
set -euo pipefail
if [ ! -d "${2}" ]; then
echo "usage: ${0} path/to/go/cmd@VERSION path/to/bin" >&2
exit 1fi
PKG="${1}"
BIN="$(cd "${2}" && pwd)"
TMP="$(mktemp -d)"trap 'rm -rf "${TMP}"' EXIT
cd "${TMP}"echo "module tmp" > go.mod
GOPATH="" GO111MODULE=on GOBIN="${BIN}" go get "${PKG}"
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/golang/go/issues/27643#issuecomment-449161211, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AE7LUS1OmJPwdQD6az1rqJzSmroYtBDyks5u7BSogaJpZM4Wlsp-
.

--
Thanks,
KomuW.
https://www.komu.engineer/about

I've been using something very similar to install Go binaries from source instead of using go get. From the examples posted it looks like we're all doing similar things to compensate for go gets behavior. https://github.com/leighmcculloch/gobin

Those of you who followed this thread might be interested in #30515, to add a "global install" command of some sort.

Another point of concern here is the unexpected removal of values from go.mod.
I am building a WAMP server that utilizes github.com/gammazero/nexus, which in turn relies in github.com/ugorji/go.  ugorji's module is atypical in that there is no code nor mod file in the root of the repo, rather everything is under /codec.
ugorji pushed a breaking change to their master branch in march 2019.  The version nexus pins in its own checked-in vendor dir is from 2018. The ugorji repo does have tagged versions, but it is impossible to actually USE them with go modules, it seems.   The code is under /codec, as is the mod file, but when attempting to pin v1.1.2 (the latest tag as of this post) via go get<EMAIL_ADDRESS>this entry is seemingly entirely ignored when executing go build, go mod vendor, or go mod tidy.  In all cases, the latest version of master is added to the sum file, subsequently used, and then even more subsequently breaks everything and causes me to see red.
The only solution I have found to this extremely idiotic issue is to do the following:
replace (
        github.com/ugorji/go/codec v0.0.0-20190204201341-e444a5086c43 => github.com/ugorji/go/codec v0.0.0-20190126102652-8fd0f8d918c8
	github.com/ugorji/go/codec v0.0.0-20190309163734-c4a1c341dc93 => github.com/ugorji/go/codec v0.0.0-20190126102652-8fd0f8d918c8
	github.com/ugorji/go/codec v0.0.0-20190315113641-a70535d8491c => github.com/ugorji/go/codec v0.0.0-20190126102652-8fd0f8d918c8
	github.com/ugorji/go/codec v0.0.0-20190316192920-e2bddce071ad => github.com/ugorji/go/codec v0.0.0-20190126102652-8fd0f8d918c8
)

require (
        github.com/ugorji/go/codec v0.0.0-20190126102652-8fd0f8d918c8 // indirect
)

This works as I basically take every version since the last tag (v1.1.2) of the repo and replace it the tagged version.
However.  When any command dealing with deps is run (get, vendor, tidy) the entry from the require block is removed for me, which then causes builds to fail with this exceptional error: build github.com/ugorji/go/codec: cannot find module for path github.com/ugorji/go/codec
I fully appreciate that I am stupid, am probably doing it wrong, etc., however it is still annoying.  I would greatly appreciate a "no cleanup" or "no edit" option or something, so I am not bit by build errors at midnight because I forgot to re-un-edit my program's mod file.

@dcarbone

When any command dealing with deps is run (get, vendor, tidy) the entry from the require block is removed for me, which then causes builds to fail with this exceptional error: build github.com/ugorji/go/codec: cannot find module for path github.com/ugorji/go/codec

go mod vendor and go mod tidy should never change your configuration from one in which everything builds to one in which something does not.
If you observe otherwise, please file a separate issue with steps to reproduce the error — this issue is closed, and closed issues are not tracked or triaged.